Skip to content
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

Update proguard keep rule for enums #1694

Merged
merged 2 commits into from
Sep 2, 2021
Merged

Update proguard keep rule for enums #1694

merged 2 commits into from
Sep 2, 2021

Conversation

romankivalin
Copy link
Contributor

📜 Description

Updated proguard rule to target library enums only.

💡 Motivation and Context

This is a consumer proguard file, which means it is included into consumer application proguard config, and if we target *, it will apply to each class of consumer application too. This unreasonably prevents obfuscation of consumer application enums.

Fixes #1684

💚 How did you test it?

Didn't test.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

I think it's a breaking change because consumer application could accidentally rely on this rule, and removing it may break obfucated code.

🔮 Next steps

@marandaneto
Copy link
Contributor

thanks for doing this @romankivalin

CHANGELOG.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #1694 (b633185) into main (eb30c90) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1694   +/-   ##
=========================================
  Coverage     75.93%   75.93%           
  Complexity     2025     2025           
=========================================
  Files           207      207           
  Lines          7060     7060           
  Branches        699      699           
=========================================
  Hits           5361     5361           
  Misses         1361     1361           
  Partials        338      338           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb30c90...b633185. Read the comment docs.

@dmytroKarataiev
Copy link

We would really appreciate it if you could notify everyone about this in similar situations in the future. Mistakes happen, but please-please try to reach out to your users.

@marandaneto
Copy link
Contributor

@dmytroKarataiev the change is part of our changelog, how would you suggest then?

@dmytroKarataiev
Copy link

@marandaneto an email would be the best, or a notification on the sentry like an outdated dependency notification but with an exclamation mark that there is a critical issue.

@marandaneto
Copy link
Contributor

@dmytroKarataiev thanks for the feedback, I will discuss that internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proguard rule for enums in sentry-android-core is too broad
4 participants