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

feat(shorebird_cli): add shorebird release get-apks command #2586

Merged
merged 7 commits into from
Oct 28, 2024

Conversation

bryanoltman
Copy link
Contributor

@bryanoltman bryanoltman commented Oct 28, 2024

Description

Adds the shorebird release get-apks command, which enables the creation of either a universal APK or spilt APKs from an existing Shorebird release.

Fixes #2423

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore
  • 🧪 Tests

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@felangel
Copy link
Contributor

felangel commented Oct 28, 2024

Sorry I didn't weigh in on this earlier but I feel like the simpler solution would've been to adjust shorebird preview to output the location of the artifact that's being run and then folks can do whatever they want with the artifact without us having to introduce a brand new command and lots of additional code that we need to maintain.

Update: opened a PR with my suggested solution at #2587

@eseidel
Copy link
Contributor

eseidel commented Oct 28, 2024

This comes up often. I'm not particularly wedded to one solution or the other (approved Felix's just because it was earlier in my inbox and I approve things sent to me).

Downloading the AAB and running bundletool are two things which are hard to do. Unclear if they should be tied together, but users don't know how to do either without our assistance. The logging solution Felix posted maybe is enough? The intent for preview might be different from the intent from this command however. Both need to have keystores in order to build the apk from an aab, the "preview" one might be using a local signing keystore, where as some other command might be using some sort of release key-store. Although one could argue if you're doing release work, why isn't it just from the original shorebird release.

@felangel
Copy link
Contributor

This comes up often. I'm not particularly wedded to one solution or the other (approved Felix's just because it was earlier in my inbox and I approve things sent to me).

Downloading the AAB and running bundletool are two things which are hard to do. Unclear if they should be tied together, but users don't know how to do either without our assistance. The logging solution Felix posted maybe is enough? The intent for preview might be different from the intent from this command however. Both need to have keystores in order to build the apk from an aab, the "preview" one might be using a local signing keystore, where as some other command might be using some sort of release key-store. Although one could argue if you're doing release work, why isn't it just from the original shorebird release.

Yeah I'm not strongly opposed to a new command was just mostly trying to offer a simpler alternative that I believe would address the customer ask 🤷

'Building apks for release ${release.version} (app: $appId)',
);
try {
await bundletool.buildApks(bundle: aabFile.path, output: apksFile.path);
Copy link
Contributor

Choose a reason for hiding this comment

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

shorebird preview has almost identical logic so it would be nice to refactor to share more code rather than having this implementation live in two separate commands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to, although the code shared between here and preview is split across several different places to accommodate both iOS and Android, and has a preview-specific cache that I was a bit hesitant to interact with here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This specific bit of code has also diverged, as preview always generates a universal APK and this command offers a way to disable that, so I'm less certain how much can be shared

@bryanoltman bryanoltman marked this pull request as ready for review October 28, 2024 20:10
@bryanoltman bryanoltman changed the title feat(shorebird_cli): add shorebird release get-apk command feat(shorebird_cli): add shorebird release get-apks command Oct 28, 2024
@bryanoltman bryanoltman merged commit 1483151 into main Oct 28, 2024
11 checks passed
@bryanoltman bryanoltman deleted the bo/get-apk-command branch October 28, 2024 20:22
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.

feat: create APK from past Shorebird release
3 participants