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 as a code owner Feb 14, 2020
@googlebot googlebot added the cla: yes 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

@anuraaga anuraaga commented Feb 14, 2020

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

Copy link
Collaborator

@elharo elharo left a comment

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
Collaborator

@elharo elharo Feb 14, 2020

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

@anuraaga anuraaga Feb 14, 2020

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

@anuraaga anuraaga commented Feb 14, 2020

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 label Feb 14, 2020
@kokoro-team kokoro-team removed the kokoro:run label Feb 14, 2020
@IanGClifton
Copy link

@IanGClifton IanGClifton commented Feb 14, 2020

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

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.

elharo
elharo approved these changes Feb 14, 2020
@elharo elharo merged commit 8a59ed0 into googleapis:master Feb 14, 2020
7 checks passed
@elharo elharo mentioned this pull request Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants