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

iOS: Add export_project_only flag #78641

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

phil-hudson
Copy link
Contributor

As per godotengine/godot-proposals#7147

This diff adds the following:

  • Add an export flag for iOS projects called 'export project only'
  • This creates the iOS project, but does not archive or produce the .ipa

This is useful for situations where there are third party build tools doing additional configuration, e.g. Fastlane

I've tested locally and everything is working okay. My only additional thought was the feature might need better naming or description.

Thanks

@phil-hudson phil-hudson requested a review from a team as a code owner June 24, 2023 08:42
@phil-hudson phil-hudson force-pushed the feat/ios_skip_ipa_export branch from 4132ea2 to fb0206f Compare June 24, 2023 11:17
@phil-hudson
Copy link
Contributor Author

should be good for review if anyone is available :)

@phil-hudson
Copy link
Contributor Author

okay, I think everything should be good now. I had to use a ternary for EditorProgress, as I couldn't hoist it, as it does not have a default constructor. Tested building on MacOS and running locally, everything worked okay.

@phil-hudson
Copy link
Contributor Author

any recommendations for an ios reviewer? ideally hoping to get it in the next release :)

@akien-mga
Copy link
Member

It's already assign for review by the iOS team, but we're in complete feature freeze for 4.1 so reviewing features isn't the priority right now.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Seems OK.

https://github.com/godotengine/godot/blob/fb7aa93c2d6a6f3f5a0de44896b67d9e65ecdc37/platform/ios/export/export_plugin.cpp#L1903-L1905

This warning shown when exporting from non-macOS system probably should be suppressed if export_project_only.

@phil-hudson
Copy link
Contributor Author

Seems OK.

https://github.com/godotengine/godot/blob/fb7aa93c2d6a6f3f5a0de44896b67d9e65ecdc37/platform/ios/export/export_plugin.cpp#L1903-L1905

This warning shown when exporting from non-macOS system probably should be suppressed if export_project_only.

It still needs MacOS for step 2 though, so I think it's okay.?

https://github.com/godotengine/godot/blob/fb7aa93c2d6a6f3f5a0de44896b67d9e65ecdc37/platform/ios/export/export_plugin.cpp#L1836-L1847

Perhaps adjusting the error might be the way? Or as both export types require MacOS to complete, should we just deny non-MacOS at the start rather than handling it later?

@phil-hudson
Copy link
Contributor Author

As this does not impact android, should we remove the android reviewer?

@akien-mga akien-mga removed the request for review from a team July 5, 2023 11:48
@akien-mga akien-mga changed the title feat(ios): add export project only flag iOS: Add export project only flag Jul 10, 2023
@akien-mga akien-mga modified the milestones: 4.x, 4.2 Jul 10, 2023
@YuriSizov YuriSizov changed the title iOS: Add export project only flag iOS: Add export_project_only flag Jul 14, 2023
@YuriSizov
Copy link
Contributor

Could you please squash your commits into one? Make sure that the final commit has a short but descriptive message (the title of this PR is a good option). See this documentation, if you need help with squashing.

@phil-hudson phil-hudson force-pushed the feat/ios_skip_ipa_export branch from fb7aa93 to c18c2d4 Compare July 16, 2023 08:42
@phil-hudson
Copy link
Contributor Author

just squashed and pushed, let me know if it's all good!

@YuriSizov
Copy link
Contributor

YuriSizov commented Jul 16, 2023

@phil-hudson Almost! You have an unrelated commit in there somehow. Also, we don't use the feat, chore, etc format. So please prefer the title of the PR instead.

@phil-hudson phil-hudson force-pushed the feat/ios_skip_ipa_export branch from c18c2d4 to 7ecc5ce Compare July 16, 2023 13:58
@YuriSizov YuriSizov force-pushed the feat/ios_skip_ipa_export branch from 7ecc5ce to 97c2170 Compare July 17, 2023 09:40
@YuriSizov
Copy link
Contributor

YuriSizov commented Jul 17, 2023

I force-pushed on your behalf to update the commit message.

Edit: Actually, I also noticed that we were missing the necessary code to translate some of the messages, so I updated that as well.

@YuriSizov YuriSizov force-pushed the feat/ios_skip_ipa_export branch from 97c2170 to 076ef3b Compare July 17, 2023 09:45
@YuriSizov YuriSizov requested review from a team as code owners July 17, 2023 09:45
@YuriSizov YuriSizov removed request for a team July 17, 2023 09:46
@YuriSizov YuriSizov merged commit cb7730c into godotengine:master Jul 17, 2023
@YuriSizov
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants