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 #63: Use auto-value for Event classes. #87

Merged
merged 1 commit into from
Nov 26, 2014
Merged

Fix #63: Use auto-value for Event classes. #87

merged 1 commit into from
Nov 26, 2014

Conversation

omo
Copy link
Contributor

@omo omo commented Nov 26, 2014

@hamidp I'm stealing this from you ;-o

@hamidp
Copy link
Contributor

hamidp commented Nov 26, 2014

@omo Thanks!

}

dependencies {
compileOnly "com.google.auto.value:auto-value:1.0-rc1+"
Copy link
Contributor

Choose a reason for hiding this comment

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

There should already be a provided configuration available here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does it mean? When there is only a dependencies section like following:

dependencies {
    provided "com.google.auto.value:auto-value:1.0-rc1+"
}

I got this error:

:sample-app:packageDebug
Error: duplicate files during packaging of APK /Users/morrita/w/RxAndroid/sample-app/build/outputs/apk/sample-app-debug-unaligned.apk
        Path in archive: META-INF/services/javax.annotation.processing.Processor
        Origin 1: /Users/morrita/.gradle/caches/modules-2/files-2.1/com.google.auto.service/auto-service/1.0-rc1/f4f5ad098bf9a3c4fc95c15214c4ea014c38ac39/auto-service-1.0-rc1.jar
        Origin 2: /Users/morrita/.gradle/caches/modules-2/files-2.1/com.google.auto.value/auto-value/1.0-rc1/95f1e38ae958b9b380846886913b1974065be454/auto-value-1.0-rc1.jar
You can ignore those files in your build.gradle:
        android {
          packagingOptions {
            exclude 'META-INF/services/javax.annotation.processing.Processor'
          }
        }
:sample-app:packageDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':sample-app:packageDebug'.
> Duplicate files copied in APK META-INF/services/javax.annotation.processing.Processor
        File 1: /Users/morrita/.gradle/caches/modules-2/files-2.1/com.google.auto.service/auto-service/1.0-rc1/f4f5ad098bf9a3c4fc95c15214c4ea014c38ac39/auto-service-1.0-rc1.jar
        File 2: /Users/morrita/.gradle/caches/modules-2/files-2.1/com.google.auto.service/auto-service/1.0-rc1/f4f5ad098bf9a3c4fc95c15214c4ea014c38ac39/auto-service-1.0-rc1.jar

What's wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the error means that there is actually a provided configuration that someone gives us but it doesn't work as we expect. It does pull auto-value related dependencies into our sample-app somehow. Hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, it comes from this guy
https://github.com/nebula-plugins/gradle-extra-configurations-plugin/blob/gradle-1.12/src/main/groovy/nebula/plugin/extraconfigurations/ProvidedBasePlugin.groovy

And it does what we don't want - Making compile depending on provided :-(

I think the workaround in this change is reasonable at this point, but probably we should talk to
nebula-plugin's team.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's fine. At the very least, you should remove the '+' from the version. Wildcard dependencies are bad news bears!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@omo
Copy link
Contributor Author

omo commented Nov 26, 2014

@JakeWharton Any chance to merge this as well?

@JakeWharton
Copy link
Contributor

Later. Out and about.
On Nov 25, 2014 9:04 PM, "Hajime Morrita" [email protected] wrote:

@JakeWharton https://github.com/JakeWharton Any chance to merge this as
well?


Reply to this email directly or view it on GitHub
#87 (comment).

}

dependencies {
compileOnly "com.google.auto.value:auto-value:1.0-rc1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Once we get this converted to an Android library (#81) we'll be able to use the apt plugin and actually see the generated source in the IDE so it doesn't appear red.

Copy link
Collaborator

Choose a reason for hiding this comment

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

JakeWharton added a commit that referenced this pull request Nov 26, 2014
Fix #63: Use auto-value for Event classes.
@JakeWharton JakeWharton merged commit 518b0cc into ReactiveX:0.x Nov 26, 2014
@mttkay
Copy link
Collaborator

mttkay commented Nov 26, 2014

Should we stick to the 2 thumbs rule before landing PRs?

@dpsm
Copy link
Contributor

dpsm commented Nov 26, 2014

For adding new functionality sounds reasonable so we have more than one
person making sure we head in the right direction feature wise.

For small fixes not sure its too much overhead?
On Nov 26, 2014 11:38 AM, "Matthias Käppler" [email protected]
wrote:

Should we stick to the 2 thumbs rule before landing PRs?


Reply to this email directly or view it on GitHub
#87 (comment).

@omo
Copy link
Contributor Author

omo commented Nov 26, 2014

@mttkay See #87 (diff)
Unfortunately nebula's provided doesn't work as expected.
I might be doing something wrong though.

@hamidp
Copy link
Contributor

hamidp commented Nov 26, 2014

@mttkay 2 thumbs up is a good rule.

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.

5 participants