Skip to content

Relax consumer rules to allow for better minifying and obfuscation#1193

Merged
tonidero merged 8 commits into
mainfrom
toniricodiez/sdk-2308-purchases-android-revisit-proguard-rules-to-be-able-to
Aug 17, 2023
Merged

Relax consumer rules to allow for better minifying and obfuscation#1193
tonidero merged 8 commits into
mainfrom
toniricodiez/sdk-2308-purchases-android-revisit-proguard-rules-to-be-able-to

Conversation

@tonidero

@tonidero tonidero commented Aug 7, 2023

Copy link
Copy Markdown
Contributor

Description

This PR relaxes the rules in our consumer-rules.pro files. This allows to obfuscate and minify more code from our SDK. Note that this means that stack traces will be obfuscated moving forward and will need the mapping file in order to deobfuscate.

@tonidero tonidero added the build label Aug 7, 2023
@codecov

codecov Bot commented Aug 7, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.92%. Comparing base (b1f7ece) to head (95364df).
Report is 538 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1193   +/-   ##
=======================================
  Coverage   85.92%   85.92%           
=======================================
  Files         181      181           
  Lines        6217     6217           
  Branches      900      900           
=======================================
  Hits         5342     5342           
  Misses        533      533           
  Partials      342      342           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tonidero tonidero marked this pull request as ready for review August 7, 2023 12:09
@tonidero tonidero requested a review from a team August 7, 2023 12:09

@vegaro vegaro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the classes annotated with @Parcelize correctly kept?

Comment thread purchases/consumer-rules.pro Outdated
@@ -1,4 +1,4 @@
-keep class com.revenuecat.** { *; }
-keep class com.revenuecat.purchases.amazon.** {*;}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this needed? We are already keeping them in the consumer rules of the amazon module ttps://github.com//pull/1193/files#diff-327934b4ae63dd824b813f85b180c4689acb3a23339ecee4dbdb40d40dec32da

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, it seems it's not needed in either place. I've tried removing it from both places and testing the app with a minified build with amazon support and it worked fine. It makes sense since we aren't accessing the Amazon SDK with reflection anymore. Removed these rules here: 93608fd

@tonidero

Copy link
Copy Markdown
Contributor Author

Are the classes annotated with @parcelize correctly kept?

From what I've seen in the official documentation, I don't think that's necessary. Parcelize works using code generation, not reflection so it should work fine even if the code/fields are minified.

Comment thread integration-tests/proguard-rules.pro Outdated
# hide the original source file name.
#-renamesourcefileattribute SourceFile
-keep class com.revenuecat.purchases.** { *; } No newline at end of file
-keep class com.revenuecat.purchases.** { *; }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we should remove these? Devs are not supposed to add this since this keeps everything, removing it from here might help us find issues if we ever have any?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, I was thinking we probably wanted to remove the integration tests module soon. But it's true this now allow us to run some basic tests against a minified version of the SDK that we wouldn't have otherwise,

@tonidero tonidero merged commit 0853414 into main Aug 17, 2023
@tonidero tonidero deleted the toniricodiez/sdk-2308-purchases-android-revisit-proguard-rules-to-be-able-to branch August 17, 2023 09:58
tonidero added a commit that referenced this pull request Aug 24, 2023
@vegaro vegaro added pr:other and removed pr:build labels Sep 17, 2024
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.

3 participants