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 ability to initialize scheme without a blueprint identifier #612

Merged
merged 4 commits into from
Jun 17, 2021

Conversation

daltonclaybrook
Copy link
Contributor

Resolves #509

Short description 📝

This PR fixes an issue where some schemes cannot be deserialized because a buildable reference does not contain a blueprint identifier.

Solution 📦

The solution is to make the blueprint field optional on BuildableReference. The other option I considered was adding a new case to Blueprint called .none, but I felt simply making the property optional would be more ergonomic.

@daltonclaybrook
Copy link
Contributor Author

Is anyone available to take a look at this? (cc: @pepibumur)

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this @daltonclaybrook

The changes look good, I have some small observations.

  • swift package generate-xcodeproj does indeed generate schemes without the blueprint identifier, though Xcode overwrites those schemes to include them when viewing them in the scheme editor. At least with this change, those generated projects and schemes can be parsed correctly.
  • This is a slightly breaking change and will need to mark it such in the CHANGELOG.md or perhaps it may need to be part of a major release - @pepibumur thoughts?
    • Example: XcodeGen relies on the public blueprintIdentifier change here

@daltonclaybrook
Copy link
Contributor Author

Any action items on my end to get this moving? (cc: @kwridan @pepibumur)

Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Hey @daltonclaybrook 👋🏼
I propose that we update the CHANGELOG.md with your changes, and flag them as breaking. I'll release a major version of XcodeProj once this is merged.

@kwridan
Copy link
Collaborator

kwridan commented Jun 15, 2021

Thanks @pepibumur! will submit a fix for tuist/tuist#2991 first thing tomorrow as it's similar in breaking nature which can be bundled in the same major release.

@kwridan
Copy link
Collaborator

kwridan commented Jun 16, 2021

@daltonclaybrook mind rebase / merging latest main with an updated change log?

@daltonclaybrook
Copy link
Contributor Author

@kwridan yep, I'll be able to take care of that later this morning.

@daltonclaybrook daltonclaybrook force-pushed the optional-blueprint-identifier branch from 22eb13e to dcb5e13 Compare June 16, 2021 21:36
@pepicrft pepicrft merged commit 5115e42 into tuist:main Jun 17, 2021
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.

Newly Generated Projects creates XCScheme without a BlueprintIdentifier
3 participants