Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix xcconfig randomly sorted generation #220

Merged
merged 7 commits into from
Apr 28, 2023

Conversation

noursandidb
Copy link
Contributor

What does this PR do

  • This PR sorts the generated xcconfig file, to make sure that it's not random every time we generate a new one, this will make things easier to see what exactly changed in the xcconfig file between variants.

How can it be tested

To test this PR, you can switch the variant, and see if the xcconfig file is sorted alphabetically.

Task

resolves #219

Checklist:

  • I ran make validation locally with success
  • I have not introduced new bugs
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new errors

@thanhtanh
Copy link

@noursandidb Thank you for your help in fixing my raised issue.
I have one suggestion, can you please check if it makes sense: I think we can keep the method getDefaultValues as it is, then modify the place we write the xcconfig file in the populateConfig(with:configFile:variant) method. Before we write to the file, we sort the dictionary by key. With this approach, we need to add a new test to check if the string content we are about to write to the file is what we expect, instead of modifying so many tests like what you did.

@noursandidb
Copy link
Contributor Author

@thanhtanh That was my first approach, but then I noticed that I wouldn't be able to test it if I'm sorting right before writing to the file.
Also I think the function should directly give me what I want, so in case we need to use that function somewhere else, we don't need to worry about re-sorting it again. we will have the same "state" (sorted) every time we call that function.

@arthurpalves
Copy link
Collaborator

I agree with @noursandidb's approach

arthurpalves
arthurpalves previously approved these changes Apr 21, 2023
@noursandidb noursandidb merged commit 029d54b into develop Apr 28, 2023
@noursandidb noursandidb deleted the fix/xcconfig-randomly-sorted-generation branch April 28, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

variants.xcconfig items should be sorted alphabetically
3 participants