Skip to content

feat(launchpad): Add asset catalog files to zip without adding to folder #2667

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

Merged
merged 2 commits into from
Aug 11, 2025

Conversation

noahsmartin
Copy link
Contributor

@noahsmartin noahsmartin commented Jul 31, 2025

This improves the app upload experience by making it so assets are written directly to the zip file rather than to the input xcarchive directory. Previously the assets were written to the input folder and then all the files in the folder were zipped. This meant the original location the user passed to the CLI had extra files in it after running the command, which is confusing.

There was also a bug that assets were not handled for ipa uploads. IPA files are already zipped, so when we wrote assets to the user's input directory we only did that for xcarchives (Which is a directory not a zip file). So a side effect of not writing images to the users input directory was also to fix this issue for IPAs, and now IPA uploads have assets included too.

@noahsmartin noahsmartin force-pushed the addAssetsToZip branch 3 times, most recently from 2a8c8f5 to 5f58d61 Compare July 31, 2025 21:11
@noahsmartin noahsmartin marked this pull request as ready for review July 31, 2025 21:16
@noahsmartin noahsmartin requested review from szokeasaurusrex and a team as code owners July 31, 2025 21:16
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@trevor-e trevor-e left a comment

Choose a reason for hiding this comment

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

The Swift changes LGTM

cursor[bot]

This comment was marked as outdated.

Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Left some questions and comments. While the PR description makes sense at a high level, I am generally a bit confused about what is meant to be changing behavior-wise here. I also don't see where the IPA file changes are happening – as this issue sounds separate based on your PR description; I would appreciate if the relevant changes for the IPA issue could be split off from the rest of the PR

cursor[bot]

This comment was marked as outdated.

@noahsmartin noahsmartin changed the base branch from master to ryan/fix_latest_clippy_lints August 8, 2025 01:19
@noahsmartin noahsmartin force-pushed the addAssetsToZip branch 2 times, most recently from c31d8b5 to 483bdb0 Compare August 8, 2025 01:23
Base automatically changed from ryan/fix_latest_clippy_lints to master August 8, 2025 08:34
@noahsmartin noahsmartin merged commit 17d95d4 into master Aug 11, 2025
30 checks passed
@noahsmartin noahsmartin deleted the addAssetsToZip branch August 11, 2025 15:19
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.

3 participants