-
Notifications
You must be signed in to change notification settings - Fork 702
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
fix: use embedded Proguard configuration instead of compile-time annotation #1491
fix: use embedded Proguard configuration instead of compile-time annotation #1491
Conversation
/cc @JakeWharton mainly since your tweet helped me out a lot :) |
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.
Looks promising. Does this change anything about Android versions supported? I may need to find an android developer to look at this.
README.md
Outdated
@@ -45,7 +45,6 @@ To use Gradle, add the following lines to your build.gradle file: | |||
```gradle | |||
repositories { | |||
mavenCentral() | |||
google() |
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 probably want to leave this here until we're ready to release 1.30.9
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.
Ah yeah - reverted!
Not sure of versions - I don't see any reason for Android API level to be affected since this is purely compilation but there might be an SDK tooling implication. @JakeWharton do you happen to know? |
I can at least confirm that it won't impact Android OS versions, but I don't know if this works when not compiling with R8. |
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.
This looks good.
You might be able to define more precise keep-rules that allow better code shrinking for apps that include this library.
But for now, it's a reasonable replacement for the AndroidX @keep annotation.
R8 provides a mechanism for embedding proguard files, which obviates the need for a compile-time dependency on an Android library in core.
Not well documented but reasonably commonly used.
https://twitter.com/jakewharton/status/1004401938467876865?lang=en
https://github.com/square/retrofit/blob/master/retrofit/src/main/resources/META-INF/proguard/retrofit2.pro
Unfortunately, without a regression test I'm not 100% sure this works, nor am I an Android developer with much confidence, just following the pattern of other libraries :)
Fixes #1487