You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/11/25 18:17:44 UTC
[kudu-CR] [java] Remove PMD static analysis
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14796
Change subject: [java] Remove PMD static analysis
......................................................................
[java] Remove PMD static analysis
This patch removes the PMD static analysis from the Gradle build.
PMD adds the the build runtime with little to no added value given
we have thorough static coverage with Checkstyle, ErrorProne, and
SpotBugs.
Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
---
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
2 files changed, 0 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/14796/1
--
To view, visit http://gerrit.cloudera.org:8080/14796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
Gerrit-Change-Number: 14796
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [java] Remove PMD static analysis
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14796 )
Change subject: [java] Remove PMD static analysis
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14796/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14796/1//COMMIT_MSG@11
PS1, Line 11: Checkstyle, ErrorProne,
> Is either of these able to detect issues like we fixed in https://github.co
SpotBugs is supposed to but didn't for some reason. I am still looking into that.
ErrorProne can detect a completely unused field, but not one that is just unwritten like peerCerts.
Regardless, PMD isn't useful for that either and it doesn't really provide any extra beneficial coverage.
--
To view, visit http://gerrit.cloudera.org:8080/14796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
Gerrit-Change-Number: 14796
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Nov 2019 20:56:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Remove PMD static analysis
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14796 )
Change subject: [java] Remove PMD static analysis
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14796/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14796/1//COMMIT_MSG@11
PS1, Line 11: Checkstyle, ErrorProne,
> SpotBugs is supposed to but didn't for some reason. I am still looking into
Thank you for the clarification. Yep, from https://pmd.github.io/ and examples it seems PMD would be an extra when we already got Checkstyle, ErrorProne and SpotBugs. Maybe, it could be useful in CPD (copy-paste detection), but I don't think that's the coverage we want to get for Kudu Java client code.
--
To view, visit http://gerrit.cloudera.org:8080/14796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
Gerrit-Change-Number: 14796
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Nov 2019 23:23:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Remove PMD static analysis
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14796 )
Change subject: [java] Remove PMD static analysis
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14796/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14796/1//COMMIT_MSG@11
PS1, Line 11: Checkstyle, ErrorProne,
Is either of these able to detect issues like we fixed in https://github.com/apache/kudu/commit/a0e896475c139d308e3b6e32110e97168b9562c6#diff-b24e070b7293ec36e80c70b6f16e1cb5R605
?
Last time I checked, SpotBugs was not able to detect that. So, I'm curious whether PMD is different in that regard.
--
To view, visit http://gerrit.cloudera.org:8080/14796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
Gerrit-Change-Number: 14796
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Nov 2019 19:05:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Remove PMD static analysis
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14796 )
Change subject: [java] Remove PMD static analysis
......................................................................
[java] Remove PMD static analysis
This patch removes the PMD static analysis from the Gradle build.
PMD adds the the build runtime with little to no added value given
we have thorough static coverage with Checkstyle, ErrorProne, and
SpotBugs.
Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
Reviewed-on: http://gerrit.cloudera.org:8080/14796
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/gradle/dependencies.gradle
M java/gradle/quality.gradle
2 files changed, 0 insertions(+), 13 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If454b4195c987a15b9c07aceef04f028ab1496d5
Gerrit-Change-Number: 14796
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)