Add support On Demand Resources #753
Conversation
There was a problem hiding this comment.
Fantastic, thank you @sipao!
I left some very minor comments above.
Could you also add a couple of things:
- a changelog entry
- an extra case here in the spec loading test https://github.com/yonaskolb/XcodeGen/blob/master/Tests/ProjectSpecTests/SpecLoadingTests.swift#L350
- add a new
$0.it("generates resource tags")test case here https://github.com/yonaskolb/XcodeGen/blob/master/Tests/XcodeGenKitTests/SourceGeneratorTests.swift - add some tags somewhere in the Test Fixture as an integration test and commit the updated project https://github.com/yonaskolb/XcodeGen/blob/master/Tests/Fixtures/TestProject/project.yml
In regards to the example in your comment, if you switch the order of the sources, you shouldn't need the excludes, as sources are processed in order, and if a previous source file has already been generated it will just use that cached version
| .map { target in | ||
| target.sources.map { $0.resourceTags }.flatMap { $0 } | ||
| }.flatMap { $0 } | ||
| )) |
There was a problem hiding this comment.
Let's sort them as well, so it's deterministic. If you add a sorted() on the end, then the Array initializer isn't needed either, as sorted returns an array
| @@ -85,6 +85,7 @@ class SourceGenerator { | |||
| let fileReference = fileReference ?? fileReferencesByPath[path.string.lowercased()]! | |||
| var settings: [String: Any] = [:] | |||
| var attributes: [String] = targetSource.attributes | |||
| let resourceTags: [String] = targetSource.resourceTags | |||
There was a problem hiding this comment.
This resourceTags property is only used once, so let's just remove it and access targetSource.resourceTags inline down below
| let projectAttributes: [String: Any] = ["LastUpgradeCheck": project.xcodeVersion] | ||
| .merged(project.attributes) | ||
| .merged(["knownAssetTags" : assetTags]) |
There was a problem hiding this comment.
Only add this if assetTags is not empty, otherwise we'll cause a diff in the project
There was a problem hiding this comment.
(This is why the tests are failing by the way)
| @@ -124,6 +125,10 @@ class SourceGenerator { | |||
| if !attributes.isEmpty { | |||
| settings["ATTRIBUTES"] = attributes | |||
| } | |||
|
|
|||
| if !resourceTags.isEmpty { | |||
| settings["ASSET_TAGS"] = resourceTags | |||
There was a problem hiding this comment.
Let's also only add these if chosenBuildPhase == .resources
| @@ -192,6 +195,7 @@ extension TargetSource: JSONObjectConvertible { | |||
|
|
|||
| createIntermediateGroups = jsonDictionary.json(atKeyPath: "createIntermediateGroups") | |||
| attributes = jsonDictionary.json(atKeyPath: "attributes") ?? [] | |||
| resourceTags = jsonDictionary.json(atKeyPath: "resourceTags") ?? [] | |||
There was a problem hiding this comment.
You'll need to add a line in toJSONValue down below as well
fixed docs Co-Authored-By: Yonas Kolb <yonaskolb@users.noreply.github.com>
|
Thanks for your review, @yonaskolb! I fixed issues you reviewed. |
Resolves #750
If the file exists directly under
FooandFoois specified in the sources, need to use excluded.