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

fix: use embedded Proguard configuration instead of compile-time annotation #1491

Merged
merged 4 commits into from Feb 14, 2020

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Feb 14, 2020

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

@anuraaga anuraaga requested a review from a team as a code owner February 14, 2020 05:17
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 14, 2020
@anuraaga anuraaga changed the title Proguard configuration instead Use embedded Proguard configuration instead of compile-time annotation Feb 14, 2020
@anuraaga anuraaga changed the title Use embedded Proguard configuration instead of compile-time annotation fix: use embedded Proguard configuration instead of compile-time annotation Feb 14, 2020
@anuraaga
Copy link
Contributor Author

/cc @JakeWharton mainly since your tweet helped me out a lot :)

Copy link
Contributor

@elharo elharo left a 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()
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah - reverted!

@anuraaga
Copy link
Contributor Author

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?

@elharo elharo added the kokoro:run Add this label to force Kokoro to re-run the tests. label Feb 14, 2020
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Feb 14, 2020
@IanGClifton
Copy link

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.

Copy link

@justcla justcla left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unresolvable dependency after 1.30.8
6 participants