You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/09/15 07:08:47 UTC
[kudu-CR] WIP: java: enable error-prone for java builds
Hello Jean-Daniel Cryans,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4425
to review the following change.
Change subject: WIP: java: enable error-prone for java builds
......................................................................
WIP: java: enable error-prone for java builds
WIP because I had to comment out some concurrency annotations due to
them catching potential bugs
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowError.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/pom.xml
4 files changed, 24 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/1
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has uploaded a new patch set (#7) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.
Fixes for the exposed issues are in follow-on commits.
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/buildSrc/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 19 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/7
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76: tasks.withType(JavaCompile) {
> that's sort of the design of error-prone. It's actually a subclass of the J
I like to have all the levers for control at the task level and often we use certain tasks for "testing" vs "packaging".
My concern is if the static code analysis complicates or breaks longer running, assembly only, packaging type jobs. For those we tend to use `gradle assemble` instead of `gradle check` or `gradle build`. If it does't break things, it likely makes the build longer.
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 26 Apr 2018 01:19:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76: tasks.withType(JavaCompile) {
> I like to have all the levers for control at the task level and often we us
Yea, that's why I think it's important to pin a specific version as I did here, so we don't have previously-working builds spontaneously break.
FWIW, Google uses error-prone by default instead of javac for all of their builds, so there's at least some evidence that it's stable and usable as a general purpose replacement. Open source bazel also does this -- java targets always use error-prone for javac.
Not sure about the build time overhead. It seemed to not be much slower as far as I could notice.
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 26 Apr 2018 17:42:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 5: Code-Review+1
Looks fine, thanks
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:30:18 +0000
Gerrit-HasComments: No
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has uploaded a new patch set (#5) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.
Fixes for the exposed issues are in follow-on commits.
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/buildSrc/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 19 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/5
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP: java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: WIP: java: enable error-prone for java builds
......................................................................
Patch Set 3:
(3 comments)
How does error-prone compare to findbugs/spotbugs?
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle
File java/build.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@24
PS3, Line 24: id 'net.ltgt.errorprone' version '0.0.13' apply false
I have been putting all of these static analysis checks into quality.gradle
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@48
PS3, Line 48: apply plugin: 'java'
Any reason you changed this to single quotes?
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@51
PS3, Line 51: apply from: "$rootDir/gradle/errorprone.gradle"
Did you add this file? I have been putting all of these static analysis checks into quality.gradle
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 25 Apr 2018 01:31:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 26 Apr 2018 17:45:21 +0000
Gerrit-HasComments: No
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76: signature 'org.codehaus.mojo.signature:java17:1.0@signature'
> I am not a fan that this needs to run all the time when compiling as oppose
that's sort of the design of error-prone. It's actually a subclass of the Java compiler that hooks into the compilation process itself, rather than doing post-compilation bytecode analysis.
I suppose it could be an optional flag, but I kind of like that it will break your compile for certain classes of errors
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:29:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has uploaded a new patch set (#6) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.
Fixes for the exposed issues are in follow-on commits.
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/6
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 3:
(3 comments)
As for error-prone vs spotbugs, I think they are likely complementary with a different set of checks. I personally like that error-prone produces its output as warnings in the normal compile output in the normal flow of development
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle
File java/build.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@24
PS3, Line 24: id 'net.ltgt.errorprone' version '0.0.13' apply false
> I have been putting all of these static analysis checks into quality.gradle
unfortunately I couldn't move the 'plugins' definition anywhere but here. It has to be the first thing in the build file apparently
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@48
PS3, Line 48: apply plugin: 'java'
> Any reason you changed this to single quotes?
mistake, reverted
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@51
PS3, Line 51: apply from: "$rootDir/gradle/errorprone.gradle"
> Did you add this file? I have been putting all of these static analysis che
moved the contents to quality.gradle
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:06:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] java: enable error-prone for java builds
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.
Fixes for the exposed issues are in follow-on commits.
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Reviewed-on: http://gerrit.cloudera.org:8080/4425
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Kudu Jenkins
---
M java/buildSrc/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 19 insertions(+), 0 deletions(-)
Approvals:
Grant Henke: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 8
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] java: enable error-prone for java builds
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 2:
> 23:08:57 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project interface-annotations: Execution default-compile of goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile failed: An API incompatibility was encountered while executing org.apache.maven.plugins:maven-compiler-plugin:3.3:compile: java.lang.UnsupportedClassVersionError: javax/tools/DiagnosticListener : Unsupported major.minor version 52.0
Something was compiled targeting JDK 8, but it's hard to tell exactly what.
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 7:
Fixed my accidental rebase back onto the old patch.
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 26 Apr 2018 17:26:03 +0000
Gerrit-HasComments: No
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4425
to look at the new patch set (#4).
Change subject: java: enable error-prone for java builds
......................................................................
java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors.
Fixes for the exposed issues are in follow-on commits.
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/build.gradle
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
3 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/4
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP: java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4425
to look at the new patch set (#3).
Change subject: WIP: java: enable error-prone for java builds
......................................................................
WIP: java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors. One of the most useful
features is that it verifies JCIP annotations like GuardedBy.
This includes some fixes to get it to pass. We should break these fixes
out into separate commits.
WIP: not quite done fixing all errors yet, and likely worth breaing out
some fixes as above
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/build.gradle
M java/gradle/dependencies.gradle
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ImportParquet.java
M java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/DeadlineTracker.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ExternalConsistencyMode.java
M java/kudu-client/src/main/java/org/apache/kudu/client/IsAlterTableDoneRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduPredicate.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowError.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RpcTraceFrame.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slice.java
M java/kudu-client/src/main/java/org/apache/kudu/util/Slices.java
M java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAuthnTokenReacquire.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestBytes.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduPredicate.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRowResult.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTestUtils.java
M java/kudu-client/src/test/java/org/apache/kudu/util/TestAsyncUtil.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
M java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java
M java/pom.xml
59 files changed, 218 insertions(+), 140 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/3
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] java: enable error-prone for java builds
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4425
to look at the new patch set (#2).
Change subject: java: enable error-prone for java builds
......................................................................
java: enable error-prone for java builds
error-prone (http://errorprone.info/) is a compiler extension by Google
which checks for a bunch of common Java errors. One of the most useful
features is that it verifies JCIP annotations like GuardedBy.
This includes some fixes to get it to pass. We should break these fixes
out into separate commits.
It also seems that building kudu-client with error-prone is extremely
slow, probably due to the generated code. We should figure out how to
avoid running it on the protobuf code.
Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowError.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Statistics.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
M java/pom.xml
6 files changed, 30 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/4425/2
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] java: enable error-prone for java builds
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/4425 )
Change subject: java: enable error-prone for java builds
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle
File java/build.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/3/java/build.gradle@24
PS3, Line 24: id 'net.ltgt.errorprone' version '0.0.13' apply false
> unfortunately I couldn't move the 'plugins' definition anywhere but here. I
This is just adding the dependencies to the build. I have been doing that in the /buildsrc repo to prevent this issue. I will post an update to this patch showing that.
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle
File java/gradle/quality.gradle:
http://gerrit.cloudera.org:8080/#/c/4425/4/java/gradle/quality.gradle@76
PS4, Line 76: tasks.withType(JavaCompile) {
I am not a fan that this needs to run all the time when compiling as opposed to as a separate task like findbugs, pmd, etc. I am not sure that is avoidable and can follow up and improve this if it is.
--
To view, visit http://gerrit.cloudera.org:8080/4425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebafbb307904875490426dd699ab1f55e21d284d
Gerrit-Change-Number: 4425
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 25 Apr 2018 22:27:02 +0000
Gerrit-HasComments: Yes