-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
chore: Improve APK Names #422
Conversation
scripts/upload-apk.sh
Outdated
@@ -37,13 +34,21 @@ fi | |||
|
|||
if [[ "$TRAVIS_BRANCH" == "$PUBLISH_BRANCH" ]]; then | |||
for file in app*; do | |||
mv ${file} badge-magic-master-${file%%} | |||
if [[ ${file} =~ "unsigned" || ${file} =~ "unaligned" ]]; then |
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.
We need unsigned dev apk
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 what do we need the unsigned apk for?
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.
We want it for the dev branch, because we don't sign dev APKs, only master ones. If you remove unsigned dev apk, no APK will be there for dev
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.
Umm no. You still will have the debug apk in the Dev branch. Also check the issue description, he specified to remove the unsigned dev apk because it is actually of no use
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.
Read the difference about debug and release apk. There'll be no release APK of development branch if you removed unsigned release APK as we don't sign it on development merge
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.
We only sign master APK, but I see no harm in doing that on development as well. If that's easy for you, go ahead
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.
Why would it be hard, just need to lift the code out of the if statements.
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.
done
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.
Why would it be hard, just need to lift the code out of the if statements.
Because that's not what it is to be done
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.
Why would it be hard, just need to lift the code out of the if statements.
Because that's not what it is to be done
why?
scripts/prep-key.sh
Outdated
if [ "$TRAVIS_PULL_REQUEST" != "false" -o "$TRAVIS_REPO_SLUG" != "fossasia/badge-magic-android" -o "$TRAVIS_BRANCH" != "$DEPLOY_BRANCH" ]; then | ||
echo "We decrypt key only for pushes to the master branch and not PRs. So, skip." | ||
exit 0 | ||
fi |
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.
You've just given everyone in the world the ability to sign any app they want with the badge-magic signing key. Congrats
There's a reason why these checks were in place
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.
lol, should have changed only the deploy branch condition. Updated
fi | ||
cp app-release-unsigned.apk app-release-unaligned.apk | ||
jarsigner -verbose -tsa http://timestamp.comodoca.com/rfc3161 -sigalg SHA1withRSA -digestalg SHA1 -keystore ../scripts/key.jks -storepass $STORE_PASS -keypass $KEY_PASS app-release-unaligned.apk $ALIAS | ||
${ANDROID_HOME}/build-tools/28.0.3/zipalign -v -p 4 app-release-unaligned.apk app-release.apk |
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.
Will fail for PRs
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.
how?
Fixes #421