Skip to content

Conversation

@AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Sep 22, 2022

Why

There were some confusion about Google Login not working on Installable Builds recently, so we want to better communicate those limitations (and other info) about Installable Builds

What

This improves the Installable Build PR comment done by CI on our PRs to:

  • Hide the QR code under a collapsible section, to reduce noise on the PR
  • Use the collapsible section and the fact that we have some room at the right of the QR code to add some more metadata there, including product flavor, and commit to build it (especially useful when you push new commits after a first installable build has been done, to check if the comment is about the old or new commit)
  • Add information about the limitations of those installable builds around Google Login (because the appID used by JalapanoDebug builds isn't whitelisted in our Firebase config at the moment — and we don't have much slots left in there to add it anyway)

[Internal ref: pdnsEh-BN-p2]

To test

Just check the look of the Installable Build comment posted by CI on this very PR.

This is how it should look like (for WordPress), collapsed by default:
image

And how it should look like (for Jetpack) once you manually expand the triangle:
image

To add extra information about flavor and limitations and make the QR code collapsed by default
@AliSoftware AliSoftware added this to the 20.9 milestone Sep 22, 2022
@AliSoftware AliSoftware self-assigned this Sep 22, 2022
@AliSoftware AliSoftware changed the title [Tooling[ Improve the Installable Build comment [Tooling] Improve the Installable Build comment Sep 22, 2022
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 22, 2022

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17199-bd865c2.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commitbd865c2
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 22, 2022

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17199-bd865c2.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commitbd865c2
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@AliSoftware AliSoftware marked this pull request as ready for review September 22, 2022 17:47
@AliSoftware AliSoftware requested review from a team and antonis September 22, 2022 17:47
@AliSoftware
Copy link
Contributor Author

PS: @antonis I've added you as reviewer since you're the one who opened #17147. So feel free to tell if you find this new wording and presentation better and if it would solve the confusion you had about Google Login earlier this month 🙂

@AliSoftware AliSoftware force-pushed the tooling/installable-build-comment-improvement branch from 4b31bb3 to 2e9ac09 Compare September 22, 2022 19:26
@ParaskP7 ParaskP7 self-assigned this Sep 23, 2022
Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Great work @AliSoftware 👍
The changes in the PR comments make things a lot clearer 🥇
The code changes also look consistent to me 🎉

Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @AliSoftware !

I reviewed and tested this PR as per the description, everything LGTM, I really love this change! 🌟 ❤️ 🥇

I have left a couple of naming related suggestion (💡) for you. I am approving this PR anyway. However, I am not merging this PR in case you would like to apply any of those suggestions.

Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

Awesome, thank you so much for addressing my feedback @AliSoftware , everything LGTM! 🚀

@AliSoftware AliSoftware merged commit f9adbfa into trunk Sep 23, 2022
@AliSoftware AliSoftware deleted the tooling/installable-build-comment-improvement branch September 23, 2022 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants