No need to keep class names

microsoft / appcenter-sdk-android

Development repository for the App Center SDK for Android

Other

276 stars 134 forks source link

No need to keep class names #1633

Closed vvb2060 closed 2 months ago

vvb2060 commented 2 years ago

[ ] Has CHANGELOG.md been updated?
[x] Are tests passing locally?
[x] Are the files formatted correctly?
[ ] Did you add unit tests?
[x] Did you test your change with either the sample apps that are included in the repository or with a blank app that uses your change?
[x] Did you check UI tests on the sample app? They are not executed on CI.

Description

keepclasseswithmembers keep class names, which is not need because we include class in arguments.

vvb2060 commented 2 years ago

Do these libraries (not even Java libraries) require reflection to use appcenter-crashes/appcenter-analytics? Why not add keep rules to their library? Most users will not use these libraries.

MatkovIvan commented 2 years ago

Unity one use it for sure.

Could you please elaborate why exactly is it important for you? For now this PR cannot be merged as-is because it requires changes in/testing of other parts of the product. But the benefit is not clear.

vvb2060 commented 2 years ago

I would like to completely obfuscate appcenter, reducing the size of the app is one reason. Another reason is that some users are very wary of telemetry, and they will suspect our app will compromise privacy because of the dex scanner warning without reading our privacy policy. I hope to mitigate this problem.

MatkovIvan commented 2 years ago

reducing the size of the app is one reason

Names of 2 classes won't help with this.

Regarding hiding "annoying" names - ok, it seems valid improvement. But as I mentioned before it requires some additional work from our team, so it cannot be merged now. Anyway, thanks for your contribution and highlighting this topic!

DmitriyKirakosyan commented 2 months ago

I'm closing this PR as stale.