-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[Tooling] Improve the Installable Build comment #17199
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
[Tooling] Improve the Installable Build comment #17199
Conversation
To add extra information about flavor and limitations and make the QR code collapsed by default
|
|||||||||||
| 💡 Scan this QR code with your Android phone to download and install the APK directly on it. | ||
| App | WordPress | |
| Build Flavor | Jalapeno | |
| Build Type | Debug | |
| Commit | bd865c2 | |
|
|||||||||||
| 💡 Scan this QR code with your Android phone to download and install the APK directly on it. | ||
| App | Jetpack | |
| Build Flavor | Jalapeno | |
| Build Type | Debug | |
| Commit | bd865c2 | |
To make it even easier to differentiate each message.
4b31bb3 to
2e9ac09
Compare
antonis
left a comment
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.
Great work @AliSoftware 👍
The changes in the PR comments make things a lot clearer 🥇
The code changes also look consistent to me 🎉
ParaskP7
left a comment
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.
👋 @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.
ParaskP7
left a comment
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, thank you so much for addressing my feedback @AliSoftware , everything LGTM! 🚀


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:
JalapanoDebugbuilds 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:

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