style: Add errorprone warnings.#592
Conversation
google-cla
Bot
added
the
cla: yes
product-auto-label
Bot
added
the
api: datastore
Capstan
force-pushed
the
errorprone
branch
3 times, most recently
from
a796401 to
585e117
Compare
|
I've adapted my original PR to use profiles to only apply ErrorProne to JDK 9 and beyond. I don't think the GraalVM test failure is my doing, as it appears to be missing some configuration file I've not touched. I believe this is ready for review. |
Capstan
force-pushed
the
errorprone
branch
3 times, most recently
from
fe8ca1b to
3077aa2
Compare
|
I believe this is again ready for review. |
|
ping |
|
Oh, so close 😮 |
kolea2
left a comment
kolea2
left a comment
There was a problem hiding this comment.
LGTM with one remaining question
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <version>3.8.0</version> | ||
| <configuration> | ||
| <source>8</source> |
There was a problem hiding this comment.
just curious if we need this since this profile is for jdk 9+ only?
There was a problem hiding this comment.
Ah, possibly not. I think I can just use
<configuration>
<release>9</release>
</configuration>
Lemme try that.
There was a problem hiding this comment.
Latest version uses that and seems to be fine with it.
There was a problem hiding this comment.
I think you can simply remove it? I was looking at https://github.com/googleapis/java-pubsublite/blob/adb22207fb3cba10de95a5e6399456d362408dd7/pom.xml#L166-L199
There was a problem hiding this comment.
I was under the understanding if you didn't have either <source> or <target> or <release>, the default behavior was that it was treated as Java 6 (i.e., 1.6), but perhaps that was in earlier releases of Java? Anyway, removing it also works.
Capstan
force-pushed
the
errorprone
branch
2 times, most recently
from
1aae18b to
9057499
Compare
Related to googleapis#590.
kolea2
added
automerge
gcf-owl-bot
Bot
removed
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
gcf-merge-on-green
Bot
removed
the
automerge
3 participants
This introduces use of the errorprone plugin as described at https://errorprone.info/docs/installation#maven. The warnings can then be addressed,
@SuppressWarnings(…)ed, or if desired, have entire classes of warnings turned off. Then once the codebase is clean, the warnings can be treated as errors. This will reduce the drift between github and the monorepo, which generally tries to enforce errorprone cleanliness.Related to #590.