Skip to content

maint(common): fix add_zip_files if flags come before zip filename#14387

Merged
ermshiperete merged 2 commits intomasterfrom
maint/common/fixzip
Jul 22, 2025
Merged

maint(common): fix add_zip_files if flags come before zip filename#14387
ermshiperete merged 2 commits intomasterfrom
maint/common/fixzip

Conversation

@ermshiperete
Copy link
Copy Markdown
Contributor

@ermshiperete ermshiperete commented Jul 22, 2025

Previously add_zip_files didn't work if the zip filename was not specified as first parameter. This change allows it to come after flags.

This will fix web release and other builds.

Test-bot: skip

Previously `add_zip_files` didn't work if the zip filename was not
specified as first parameter. This change allows it to come after flags.

This will fix web release builds.

Test-bot: skip
@keymanapp-test-bot keymanapp-test-bot bot added this to the A19S8 milestone Jul 22, 2025
@github-actions github-actions bot added resources/ common/ maint Maintenance work -- continuous integration, build scripts, infrastructure labels Jul 22, 2025
ZIP_FILE="$1"
else
INCLUDE+=("$1")
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah, I think this will fix the Android build too.
Can you update the header comments ll. 11-12? (and maybe you took care of the TODO on line 9)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually, if the zip filename needs to be after the flags now, I think the android/build.sh will need to get updated too

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... and ios/tools/prepRelease.sh ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The flags can come anywhere now. Added unit tests that show that.

Also updated the function header and removed the TODO (saw that yesterday but then forgot to remove...). Thanks!

- update doc in the function header
- add unit tests to show that flags are allowed anywhere now
Base automatically changed from test/common/ziptests to master July 22, 2025 15:15
@ermshiperete ermshiperete merged commit fba8797 into master Jul 22, 2025
29 checks passed
@ermshiperete ermshiperete deleted the maint/common/fixzip branch July 22, 2025 15:16
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Jul 22, 2025
@keyman-server
Copy link
Copy Markdown
Collaborator

Changes in this pull request will be available for download in Keyman version 19.0.87-alpha

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

Labels

common/ maint Maintenance work -- continuous integration, build scripts, infrastructure resources/

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants