-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support Error Prone on Java 8-11 #32
Conversation
@@ -787,52 +803,35 @@ | |||
<!-- We enable nearly all doclint checks, | |||
except that we don't care about missing Javadoc | |||
on non-public classes and members. --> | |||
<arg>-Xdoclint:all</arg> | |||
<arg>-Xdoclint:missing/protected</arg> | |||
<arg>-Xdoclint:all,-missing/package</arg> |
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 change is not just "cleanup": the previous two-argument variant no longer works on JDK 11.
4c4c3dd
to
db543cb
Compare
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.
Nice work!
Only tested that it still compiles for me on OpenJDK8.
Question: how does this relate to PRP-8197?
.travis.yml
Outdated
# XXX: On JDK 10 Error Prone causes warnings to be emitted; see | ||
# https://github.com/google/error-prone/issues/860. | ||
script: ./mvnw install -Dverification.warn | ||
script: ./mvnw install | ||
- jdk: openjdk-ea |
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.
Should we include oraclejdk-ea
, too?
It also seems that openjdk11
is now available. (https://docs.travis-ci.com/user/languages/java/#using-java-10-and-later)
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.
Should we include
oraclejdk-ea
, too?
Well, see #30 and travis-ci/travis-ci#9963 ;)
It also seems that
openjdk11
is now available.
Good one; will try that.
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.
Just tested; oraclejdk-ea
still fails. Added oraclejdk11
and openjdk11
:)
3873856
to
d9373f6
Compare
841d9c7
to
fa712ce
Compare
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.
Verified that it runs with Oracle JDK 10
d9373f6
to
01a0c83
Compare
b5d70d6
to
d96d313
Compare
01a0c83
to
40c0e94
Compare
The previous Error Prone setup only properly worked on Java 8 and 9. By configuring Error Prone as a javac plugin it now also successfully runs on Java 10 and above. Running the plugin under Java 8 requires a modest amount of extra configuration. While there: - Enable the `ConstructorLeaksThis` check now that the bug that was filed against it is fixed. - Drop the `-Xlint:-processing` flag because it seems the code now compiles fine without it.
And move flags that only have an impact when forking to the relevant location.
d96d313
to
e7e4a8c
Compare
The previous Error Prone setup only properly worked on Java 8 and 9. By configuring Error Prone as a javac plugin it now also successfully runs on Java 10 and above. Running the plugin under Java 8 requires a modest amount of extra configuration.
While there:
ConstructorLeaksThis
check now that the bug that was filed against it is fixed.-Xlint:-processing
flag because it seems the code now compiles fine without it.This PR is on top of #31. @imTachu, for various reasons upgrading our internal build will require more work than what is shown here, but as you can see proper Java 10+ support is inching closer :).
NB: the work in this PR proved not quite trivial; I opened google/error-prone#1115 so that it may be easier for the next person to try this.