-
Notifications
You must be signed in to change notification settings - Fork 25
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
Switch to incap helper #114
Conversation
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.
I approve the code changes. Verified them in the auto-value-moshi-tests
project (make a small change to a source file, ./gradlew :auto-value-moshi-tests:compileJava --debug --no-build-cache
–fwiw, I enable build cache globally– and look for Full recompilation
and/or Recompiled classes
in the output; note that I added net.ltgt.gradle.incap:incap:0.2
as an explicit annotationProcessor
dependency to workaround google/auto#615 (comment)).
BTW, shouldn't you "shade" dependencies? I mean, at least auto-common.
auto-value-moshi/build.gradle
Outdated
@@ -1,15 +1,20 @@ | |||
apply plugin: 'net.ltgt.apt' |
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.
You should revert that change, and use annotationProcessor
.
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.
So when I use that, it fails on CI due to not finding generated classes - https://travis-ci.org/rharter/auto-value-moshi/builds/456470849?utm_source=github_status&utm_medium=notification
I tried this just in case, and sure enough it did seem to fix it. Any sense on why it only happens in CI?
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.
That was because auto-service wasn't in the processor path anymore at that commit.
At a minimum you could revert the annotationProcessor
→ apt
change, whether using net.ltgt.apt
or not (it will setup options.annotationProcessorGeneratedSourcesDirectory
, but this is useless here as those processors only generate resources; see the project's README to determine whether it's useful for you or not, in this specific case, it's not).
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.
auto-value-moshi/build.gradle
Outdated
apply plugin: 'java' | ||
apply plugin: 'maven-publish' | ||
|
||
sourceCompatibility = versions.java | ||
targetCompatibility = versions.java | ||
|
||
dependencies { | ||
apt "net.ltgt.gradle.incap:incap-processor:0.2" |
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.
Move to dependencies.gradle
?
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.
Will do
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.
Thanks for the details on how to verify, I wish the docs had a bit more information on that :|
As opposed to the current
Yes, planning to look at that in a followup. Filed #115 |
That was in the auto-value-moshi-tests subproject. If you don't add it, you'll see:
in the |
Nevermind, I can't read apparently and just understood your last comment |
No description provided.