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

Set higher version .xcdatamodeld file as current #341

Merged
merged 3 commits into from
Jul 18, 2018

Conversation

rohitpal440
Copy link
Contributor

Sort .xcdatamodeld files in decreasing order to set higher version core data model as current.

Current sorting of file takes place as:

ABC 10.xcdatamodel
ABC 11.xcdatamodel
ABC 12.xcdatamodel
ABC 13.xcdatamodel
ABC 14.xcdatamodel
ABC 15.xcdatamodel
ABC 16.xcdatamodel
ABC 17.xcdatamodel
ABC 18.xcdatamodel
ABC 19.xcdatamodel
ABC 2.xcdatamodel
ABC 20.xcdatamodel
ABC 3.xcdatamodel
ABC 4.xcdatamodel
ABC 5.xcdatamodel
ABC 6.xcdatamodel
ABC 7.xcdatamodel
ABC 8.xcdatamodel
ABC 9.xcdatamodel

after this change sorting will be like

ABC 20.xcdatamodel
ABC 19.xcdatamodel
ABC 18.xcdatamodel
ABC 17.xcdatamodel
ABC 16.xcdatamodel
ABC 15.xcdatamodel
ABC 14.xcdatamodel
ABC 13.xcdatamodel
ABC 12.xcdatamodel
ABC 11.xcdatamodel
ABC 10.xcdatamodel
ABC 9.xcdatamodel
ABC 8.xcdatamodel
ABC 7.xcdatamodel
ABC 6.xcdatamodel
ABC 5.xcdatamodel
ABC 4.xcdatamodel
ABC 3.xcdatamodel
ABC 2.xcdatamodel

and First element will be set as current core data model

To set higher version core data  model as current
@yonaskolb
Copy link
Owner

Thanks @rohitpal440. You will probably need to apply the same logic here as well, as all the groups are sorted again before generation https://github.com/rohitpal440/XcodeGen/blob/b0fa7a2845213aeeaa4caf07ad30eb060bfdf00c/Sources/XcodeGenKit/PBXProjGenerator.swift
If it’s possible to add a test as well that would be great!

@rohitpal440
Copy link
Contributor Author

@yonaskolb
Copy link
Owner

Yes, specifically this line https://github.com/rohitpal440/XcodeGen/blob/b0fa7a2845213aeeaa4caf07ad30eb060bfdf00c/Sources/XcodeGenKit/PBXProjGenerator.swift#L291
Sorry, I guess the line number didn't copy across on my phone properly

@@ -119,7 +119,7 @@ class SourceGenerator {
let models = (try? path.children()) ?? []
let modelFileReference = models
.filter { $0.extension == "xcdatamodel" }
.sorted()
.sorted { $0.string.localizedStandardCompare($1.string) == .orderedDescending }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this will be rewritten in sortGroups shouldn't this be .orderedAscending?

Copy link
Contributor Author

@rohitpal440 rohitpal440 Jul 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I've to do something like this currentVersion: modelFileReference.last?.reference over here https://github.com/rohitpal440/XcodeGen/blob/4d964c07861cd524801b85d4edf611c77e1d2c3b/Sources/XcodeGenKit/SourceGenerator.swift#L134
to make higher version of .xcdatamodel file as current

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if they are sorted alphabetically then it should use the last one 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what do you suggest me to do?

  1. Keep the code as in this PR
    OR
  2. Compare using .orderedAscending and update the code for getting currentVersion to currentVersion: modelFileReference.last?.reference

In both ways I'm going to get the same result, chose approach 1 since the change stays in one place instead of 2.

Copy link
Owner

@yonaskolb yonaskolb Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go with option 2. As you want to sort in ascending order

Copy link
Owner

@yonaskolb yonaskolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thank you so much @rohitpal440!

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.

2 participants