-
Notifications
You must be signed in to change notification settings - Fork 38
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
Create and upload universal APK during release #190
Conversation
11252ee
to
d094439
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome!!
@@ -32,6 +32,9 @@ jobs: | |||
with: | |||
flutter-version: '3.24.1' | |||
|
|||
- name: Setup bundletool for APK generation | |||
uses: amyu/setup-bundletool@f7a6fdd8e04bb23d2fdf3c2f60c9257a6298a40a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to nitpick here, but https://github.com/mukeshsolanki/bundletool-action looks a little more active.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some differences - mukeshsolanki downloads the latest bundletool or a specified tag, amyu always downloads 1.15.1.
mukeshsolanki might have a CI warning about using an old node version.
mukeshsolanki is configured a bit differently.
I would need to go read about how CI Actions work, but mukeshsolanki seems to be able to run bundletool for you:
https://github.com/mukeshsolanki/bundletool-action/blob/main/src/action.js#L37-L43
Versus:
https://github.com/amyu/setup-bundletool/blob/main/src/main.ts#L28-L34
Anyway, I don't feel too strongly. I went to go look because using a SHA instead of a version here wasn't typical, and saw that there are no stars or forks. mukeshsolanki is listed first on the Action store with a few stars: https://github.com/marketplace/actions/bundletool-action
So I think it's a wash! But maybe we can stick a comment about why we're using the SHA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did look at that one briefly, but it requires specifying signing credentials, which made me a bit nervous, and does more than just installing bundletool so I have a feeling it might be less flexible. This action is really simple and gives us the flexibility to use bundletool however we want, which is why I went with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For actions that are at all sketchy (not very used, uncommon, etc), I like to use a SHA just to avoid them from releasing something nefarious and us automatically using it. It just gives us more control over the code that is being run. I looked over the source code of that commit, so I feel alright using it as-is.
This will give users a way to sideload the app without using Google Play or having to deal with running
bundletool
.Closes #150.