Skip to content

HBASE-19762 Fixed Checkstyle errors in hbase-http #131

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

Merged
merged 1 commit into from
Apr 11, 2019

Conversation

HorizonNet
Copy link
Contributor

No description provided.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
0 reexec 273 Docker mode activated.
_ Prechecks _
+1 hbaseanti 0 Patch does not have any anti-patterns.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 10 new or modified test files.
_ master Compile Tests _
0 mvndep 24 Maven dependency ordering for branch
+1 mvninstall 254 master passed
+1 compile 32 master passed
+1 checkstyle 136 master passed
+1 shadedjars 264 branch has no errors when building our shaded downstream artifacts.
0 findbugs 0 Skipped patched modules with no Java source: hbase-checkstyle
+1 findbugs 32 master passed
+1 javadoc 26 master passed
_ Patch Compile Tests _
0 mvndep 16 Maven dependency ordering for patch
+1 mvninstall 249 the patch passed
+1 compile 31 the patch passed
+1 javac 31 the patch passed
+1 checkstyle 133 root: The patch generated 0 new + 0 unchanged - 143 fixed = 0 total (was 143)
+1 whitespace 0 The patch has no whitespace issues.
+1 xml 3 The patch has no ill-formed XML file.
+1 shadedjars 269 patch has no errors when building our shaded downstream artifacts.
+1 hadoopcheck 518 Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
0 findbugs 0 Skipped patched modules with no Java source: hbase-checkstyle
+1 findbugs 40 the patch passed
+1 javadoc 26 the patch passed
_ Other Tests _
+1 unit 12 hbase-checkstyle in the patch passed.
+1 unit 56 hbase-http in the patch passed.
+1 asflicense 22 The patch does not generate ASF License warnings.
2506
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/1/artifact/out/Dockerfile
GITHUB PR #131
Optional Tests dupname asflicense checkstyle javac javadoc unit xml shadedjars hadoopcheck compile findbugs hbaseanti
uname Linux 055b27d5ba5f 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:16:02 UTC 2018 x86_64 GNU/Linux
Build tool maven
Personality /testptch/patchprocess/precommit/personality/provided.sh
git revision master / 89ce5d1
maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
Default Java 1.8.0_181
findbugs v3.1.11
Test Results https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/1/testReport/
Max. process+thread count 281 (vs. ulimit of 10000)
modules C: hbase-checkstyle hbase-http U: .
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/1/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@@ -37,6 +37,7 @@
<suppress checks="VisibilityModifier" files=".*/src/test/.*\.java"/>
<suppress checks="InterfaceIsTypeCheck" files=".*/src/main/.*\.java"/>
<suppress checks="EmptyBlockCheck" files="TBoundedThreadPoolServer.java"/>
<suppress checks="EmptyBlockCheck" files=".*/src/test/.*\.java"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe too general? We disable this check in our the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. That's too general. Will change it.

@@ -1,4 +1,4 @@
/**
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be **?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time (example) I see it with a single *. I know that we use some kind of mixture. In IntelliJ this is also marked as a dangling JavaDoc comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think in HBase we usually use two stars, and also in hadoop? At least, our code template under the dev-support uses two stars.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let me change it back to **.

@@ -1,4 +1,4 @@
/**
/*
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the empty line?

@HorizonNet HorizonNet requested a review from Apache9 April 9, 2019 21:18
@HorizonNet
Copy link
Contributor Author

@Apache9 I updated the PR according to your feedback. Thanks for that.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
0 reexec 74 Docker mode activated.
_ Prechecks _
+1 hbaseanti 0 Patch does not have any anti-patterns.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 10 new or modified test files.
_ master Compile Tests _
0 mvndep 45 Maven dependency ordering for branch
+1 mvninstall 333 master passed
+1 compile 27 master passed
+1 checkstyle 130 master passed
+1 shadedjars 308 branch has no errors when building our shaded downstream artifacts.
0 findbugs 0 Skipped patched modules with no Java source: hbase-checkstyle
+1 findbugs 70 master passed
+1 javadoc 68 master passed
_ Patch Compile Tests _
0 mvndep 33 Maven dependency ordering for patch
+1 mvninstall 280 the patch passed
+1 compile 28 the patch passed
+1 javac 28 the patch passed
+1 checkstyle 131 root: The patch generated 0 new + 0 unchanged - 143 fixed = 0 total (was 143)
+1 whitespace 0 The patch has no whitespace issues.
+1 xml 6 The patch has no ill-formed XML file.
+1 shadedjars 261 patch has no errors when building our shaded downstream artifacts.
+1 hadoopcheck 507 Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
0 findbugs 0 Skipped patched modules with no Java source: hbase-checkstyle
+1 findbugs 38 the patch passed
+1 javadoc 28 the patch passed
_ Other Tests _
+1 unit 12 hbase-checkstyle in the patch passed.
+1 unit 53 hbase-http in the patch passed.
+1 asflicense 20 The patch does not generate ASF License warnings.
2586
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/2/artifact/out/Dockerfile
GITHUB PR #131
Optional Tests dupname asflicense checkstyle javac javadoc unit xml shadedjars hadoopcheck compile findbugs hbaseanti
uname Linux e630cb7b8a9e 4.4.0-137-generic #163-Ubuntu SMP Mon Sep 24 13:14:43 UTC 2018 x86_64 GNU/Linux
Build tool maven
Personality /testptch/patchprocess/precommit/personality/provided.sh
git revision master / 387a5da
maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
Default Java 1.8.0_181
findbugs v3.1.11
Test Results https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/2/testReport/
Max. process+thread count 291 (vs. ulimit of 10000)
modules C: hbase-checkstyle hbase-http U: .
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/2/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@HorizonNet
Copy link
Contributor Author

@Apache9 Currently trying to figure out GitHubs merge capabilities. Normally we use Rebase and merge, but this time we have two commits. Should I use Sqash and merge instead or do we have another approach for handling such cases?

@Apache9
Copy link
Contributor

Apache9 commented Apr 11, 2019

Please do a force push to your own branch to merge the two commits into one first, and then use 'Rebase and merge'.

There is a discussion thread on the dev list about disabling squash and merge so we'd better not use it for now.

@HorizonNet HorizonNet merged commit fc6e3fc into apache:master Apr 11, 2019
@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 0 Docker mode activated.
-1 patch 7 #131 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.
Subsystem Report/Notes
GITHUB PR #131
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-131/3/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants