-
Notifications
You must be signed in to change notification settings - Fork 313
Fix ProGuard R8 issues on PasscodeLock library (release/2.36) #1758
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
Conversation
FYI: As part of the below PR#1678, the author most probably copy-pasted an app module's 'build.gradle' configuration into this library module. And with that, the author also copy-pasted this 'buildTypes.release' related configuration by an oversight. 'PasscodeLock-Android' never had such a configuration to begin with, and it being a library, it actually didn't need it. PasscodeLock-Android: https://github.com/wordpress-mobile/ PasscodeLock-Android Related: [Lib] Migrate passcodelock directly into simplenote as a library module. #1678 - #1678 PS: An alternative would have been to add a 'consumer-rules.pro' file to have some extra ProGuard configuration for this library. But again, 'PasscodeLock-Android' never had such a need.
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.
Pull Request Overview
This PR fixes a release build failure in the assembleRelease R8 build process by removing ProGuard configuration from the PasscodeLock module. The fix involves removing the release build type configuration that enabled minification and referenced ProGuard rules, as well as deleting the ProGuard rules file entirely.
- Removed release build type configuration with minification settings from PasscodeLock module
- Deleted the ProGuard rules file that was causing build issues
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| PasscodeLock/build.gradle | Removed release buildType configuration that enabled minification and ProGuard |
| PasscodeLock/proguard-rules.pro | Deleted entire ProGuard rules file |
Generated by 🚫 Danger |
|
📲 You can test the changes from this Pull Request in Simplenote Android by scanning the QR code below to install the corresponding build.
|
AliSoftware
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.
- Ran
./gradlew assembleReleaseonrelease/2.36and confirmed it failed - Ran
./gradlew assembleReleaseon thisfix/release/2.36branch from this PR and confirmed it fixes 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.
Looking at the git log of this file I see only one commit in history, the one introducing it… that was on Aug 12… but wait, we’re Aug 11, how is that possible… oh. wait. it was on Aug 12… 2024! 😓
I guess the only reason we never noticed the issue before… is because we never made a release build of SNAndroid since a whole year?… 😅
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’re Aug 11, how is that possible… oh. wait. it was on Aug 12… 2024!
That was such a coincidence, heh
I guess the only reason we never noticed the issue before… is because we never made a release build of SNAndroid since a whole year?… 😅
this! exactly looks like so
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 guess the only reason we never noticed the issue before… is because we never made a release build of SNAndroid since a whole year?… 😅
Yea, no @AliSoftware , I also looked at that and I am not sure, the timelines still don't quite much... 🤷
- This [Lib] Migrate passcodelock directly into simplenote as a library module. #1678 PR got merged to
trunkon Aug 14, 2024. - Then, 2+ months later, Simplenote's latest release (
2.35) got updated on Oct 25, 2024 (Playstore -> About this app). - Also, while looking on the Play Console, I am seeing this
2.35release on the production track, last updated on Nov 11, 2024 (3 months later).
This made me think that we indeed made a release build of SNAndroid after #1678. I am just not sure how come this problem didn't exist back then, and came to assume that this was (most probably) because of all the AGP updates we made since 2.35... 🤷
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.
Btw, thanks for the PR title change @AliSoftware , much much better... 😊
Fix
This change fixes the
assembleReleaseR8build failure onpasscodelock.For more info see commit's 6086403 description.
Test
Run
assembleReleaseand make sure it builds as expected.Review
Release