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

Add support for customizing attributes on a target source file #583

Merged
merged 6 commits into from Jun 14, 2019

Conversation

min
Copy link
Contributor

@min min commented May 8, 2019

When adding to an intents definition file to a target, Xcode will do some code gen at build time. In some cases we need to add the definition file to multiple targets (shared frameworks), but only have the code generation happen for one of the targets to avoid duplicated code.

Per apple docs:

To use a custom intent in your app, Xcode needs to generate source code for the items defined in the Intent Definition file. However, for apps that have a shared framework, like Soup Chef, code generation should only happen for the framework target. Generating the code for the shared framework and app causes conflicts due to duplicated code.
To specify which target has the generated source code, use the Intent Classes setting in the Target Membership panel. For Soup Chef, only the SoupKit target uses the Intent Classes setting; all other targets use the No Generated Classes setting. And because SoupKit shares its code with other targets, the other targets can use the order soup intent.

Screen Shot 2019-05-08 at 6 52 18 AM

This adds a noCoden property to target source which internal sets settings = {ATTRIBUTES = (no_codegen, ); }; on the intents definition PBXBuildFile.

@min
Copy link
Contributor Author

min commented May 8, 2019

@yonaskolb this is the last remaining issue after migrating our app to xcodegen. not sure if this is the correct solution, would love sone feedback .

@yonaskolb
Copy link
Owner

Thanks for the PR @min!
Perhaps another way to approach this would be to allow custom attributes on a TargetSource in the form of [String: Any], similar to Target.
That would allow for more flexibility going forward. I would probably prefer not to implement every possible attribute manually like this, though I do know it makes it a bit more user friendly. What do you think?

@min min changed the title Add support for no_codegen settings ATTRIBUTE Allow customization of attributes on a target source file May 11, 2019
@min
Copy link
Contributor Author

min commented May 11, 2019

@yonaskolb That makes sense, changed to a [String] as that's what's expected for ATTRIBUTES.

@min min changed the title Allow customization of attributes on a target source file Add support for customizing attributes on a target source file May 11, 2019
@min
Copy link
Contributor Author

min commented Jun 13, 2019

@yonaskolb would love to get this merged in as we're using a forked version for now :)

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.

Sorry for the delay @min, I've just been so busy. I left a single comment. Could you also add a changelog entry? Then we can get this merged 👍

Docs/ProjectSpec.md Outdated Show resolved Hide resolved
min and others added 3 commits June 13, 2019 21:30
@min
Copy link
Contributor Author

min commented Jun 14, 2019

@yonaskolb all set

CHANGELOG.md Outdated
@@ -2,10 +2,9 @@

## Next Version

#### Added
Copy link
Owner

Choose a reason for hiding this comment

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

Was removing this heading an accident? Let's keep it around. It's fine to remove the blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yonaskolb that was a merge error, should be fixed now.

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.

Great, thanks for this @min!

@yonaskolb yonaskolb merged commit 2ccc7df into yonaskolb:master Jun 14, 2019
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.

None yet

2 participants