Skip to content

Allow client to customize buildFor in custom Xcode schemes #901

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 27 commits into from
Aug 10, 2022
Merged

Allow client to customize buildFor in custom Xcode schemes #901

merged 27 commits into from
Aug 10, 2022

Conversation

cgrindel
Copy link
Contributor

@cgrindel cgrindel commented Aug 9, 2022

Related to #573.

  • A client now provides XcodeScheme.BuildTarget values to XcodeScheme.BuildAction. The build target provides a means for specifying individual buildFor settings.
  • Implemented XcodeScheme.withDefautls. This computed property returns a copy of theXcodeScheme with the appropriate defaults applied (e.g. default actions, relevant buildFor settings).
  • Added validation checks to the initializers for XcodeScheme, XcodeScheme.BuildAction, and XcodeScheme.TestAction.

@cgrindel cgrindel requested a review from brentleyjones August 9, 2022 21:18
@cgrindel cgrindel self-assigned this Aug 9, 2022
@cgrindel cgrindel mentioned this pull request Aug 9, 2022
Closed
37 tasks
brentleyjones
brentleyjones reviewed Aug 9, 2022
View reviewed changes
cgrindel added 24 commits August 9, 2022 16:53
@cgrindel
Save work
ad4a41f
@cgrindel
Setting buildFor specified via XcodeScheme.
4197334
@cgrindel
Fix merge issues
1296d37
@cgrindel
Add check in BuildTarget to ensure duplicate labels are not provided.
7729969
@cgrindel
Tests are green with BuildActionInfo init setting buildFor bits.
50158bf
@cgrindel
Comvinded action defaults and buildFor merging
4712a57
@cgrindel
Fix BuildAction custom init test
5039054
@cgrindel
Fix XCSchemeInfo custom scheme init test
965e204
@cgrindel
Add withDefaults test: noBuild, withLaunch, withProfile
eb60053
@cgrindel
Add withDefaults test: withBuild, withLaunch, noProfile,
b1425b9 
profilingDisabled
@cgrindel
Rework profileaction logic
b806419
@cgrindel
Check to ensure that at least one target was specified in a scheme
2a4fbd2
@cgrindel
Add validity checks in XcodeScheme, XcodeScheme.TestAction
65f5c28
@cgrindel
Add test placeholders
636d97f
@cgrindel
Fix broken test
460c842
@cgrindel
Add better error message when BuildFor value merge fails
e5347db
@cgrindel
Fleshed out the merge error message a bit more
1909b2c
@cgrindel
Ensure that at least one target is marked as running.
dff9414
@cgrindel
Add test for no targets passed to BuildAction init
6a628fd
@cgrindel
Add test for no actions passed to XcodeScheme init
87ba4c6
@cgrindel
Fix merge issue
1b376bb
@cgrindel
Clean up.
f04b6cd
@cgrindel
Switch scheme buildFor error to UsageError
24e98ff
@cgrindel
Add assertPreconditionError
b5010fc
@cgrindel cgrindel force-pushed the cg/gh573_add_build_target_to_xcodescheme branch from b1649b5 to b5010fc Compare August 9, 2022 22:53
@cgrindel
Copy link
Contributor Author

cgrindel commented Aug 9, 2022

@brentleyjones The integration test does not seem to be kicking-off in CI. Any thoughts?

@cgrindel cgrindel requested a review from brentleyjones August 9, 2022 22:54
brentleyjones
brentleyjones reviewed Aug 9, 2022
View reviewed changes
@brentleyjones
Copy link
Contributor

Let's see if it fixes itself after your next push. If not I'll debug.

@cgrindel cgrindel requested a review from brentleyjones August 10, 2022 04:00
brentleyjones
brentleyjones approved these changes Aug 10, 2022
View reviewed changes
Copy link
Contributor

@brentleyjones brentleyjones left a comment

Choose a reason for hiding this comment

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

Shouldn't this PR have changed the generator scheme to not have tests have the running buildFor?

@cgrindel
Copy link
Contributor Author

Shouldn't this PR have changed the generator scheme to not have tests have the running buildFor?

Good catch. I ran fixtures update and it did update the settings. Do we need to update the tests to detect changes in scheme files?

@cgrindel cgrindel enabled auto-merge (squash) August 10, 2022 13:45
@cgrindel
Copy link
Contributor Author

@brentleyjones It looks like Integration Test is not reporting back to GitHub. Thoughts don't how to debug?

Also, I realized that I never update the Starlark to support the new build targets. I am creating a PR for that now.

@brentleyjones
Copy link
Contributor

Do we need to update the tests to detect changes in scheme files?

They should detect changes already, as it simply diffs the whole bundle folder. If they aren't for some reason, yes, we need to fix that.

It looks like Integration Test is not reporting back to GitHub. Thoughts don't how to debug?

It's a bug on the Workflows side. We are looking into it.

@brentleyjones brentleyjones disabled auto-merge August 10, 2022 13:57
@brentleyjones brentleyjones merged commit 683ef70 into main Aug 10, 2022
@brentleyjones brentleyjones deleted the cg/gh573_add_build_target_to_xcodescheme branch August 10, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brentleyjones brentleyjones brentleyjones approved these changes

Assignees

@cgrindel cgrindel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cgrindel @brentleyjones