You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/01/24 16:57:24 UTC
[GitHub] [ozone] lamber-ken opened a new pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
lamber-ken opened a new pull request #1839:
URL: https://github.com/apache/ozone/pull/1839
## What changes were proposed in this pull request?
Add ModifierOrder to checkstyle rules
- `public final static` to `public static final`
- `static public` to `public static`
- etc
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4737
## How was this patch tested?
No need new tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-770200953
> Thanks @lamber-ken for the improvement, @amaliujia and @ayushtkn for the review.
Welcome.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1839:
URL: https://github.com/apache/ozone/pull/1839
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-766450485
+1 seems be fine to enforce modifier order from now. The existing violation thus fix in this PR is not too large.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-770195225
Thanks @lamber-ken for the improvement, @amaliujia and @ayushtkn for the review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken edited a comment on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-769817929
hi @adoroszlai , please review this pr again if you are free, thanks. If the merge operation is late, it will cause more conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ayushtkn commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-767011190
Well this looks good to have, but this is changing the rules at the project level, I think such changes if required should be notified on the dev list.
FYI. Hadoop too had this checkstyle rule, but was disabled as part of [HADOOP-12713](https://issues.apache.org/jira/browse/HADOOP-12713)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on a change in pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on a change in pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#discussion_r567222032
##########
File path: pom.xml
##########
@@ -1974,6 +1974,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
<suppressionsLocation>hadoop-hdds/dev-support/checkstyle/suppressions.xml</suppressionsLocation>
<includeTestSourceDirectory>true</includeTestSourceDirectory>
<failOnViolation>false</failOnViolation>
+ <consoleOutput>true</consoleOutput>
Review comment:
Got, updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-767217696
Got @amaliujia @ayushtkn
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-767027453
Sounds reasonable to at least mention this in dev list as it will impact all developers (though the impact is supposed to be minor)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-770174295
> There is no objection on the [MailingList](https://lists.apache.org/thread.html/ra82429d3ae977ec1b5b0b2b194d74fba040765386751bcffd5b3e26a%40%3Cdev.ozone.apache.org%3E)
> We are good to go then. +1
Thanks 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-770157593
hi @adoroszlai , please review this pr again if you are free, thanks. If the merge operation is late, it will cause more conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-769817929
hi @amaliujia @ayushtkn @elek, please review this pr again if you are free, thanks. If the merge operation is late, it will cause more conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] amaliujia commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-766450485
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-768212371
hi @amaliujia @ayushtkn @elek, please review this pr again if you are free, thanks 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken edited a comment on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-767217696
Got. @amaliujia @ayushtkn
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken edited a comment on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-767217696
Got. @amaliujia @ayushtkn
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken removed a comment on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken removed a comment on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-769817929
hi @adoroszlai , please review this pr again if you are free, thanks. If the merge operation is late, it will cause more conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-766962701
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken edited a comment on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-769817929
hi @amaliujia @ayushtkn @elek @adoroszlai , please review this pr again if you are free, thanks. If the merge operation is late, it will cause more conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lamber-ken commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-766962701
> +1 seems be fine to enforce modifier order from now. The existing violation thus fix in this PR is not too large.
Thanks @amaliujia
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ayushtkn commented on pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#issuecomment-767011190
Well this looks good to have, but this is changing the rules at the project level, I think such changes if required should be notified on the dev list.
FYI. Hadoop too had this checkstyle rule, but was disabled as part of [HADOOP-12713](https://issues.apache.org/jira/browse/HADOOP-12713)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #1839: HDDS-4737. Add ModifierOrder to checkstyle rules
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1839:
URL: https://github.com/apache/ozone/pull/1839#discussion_r567214527
##########
File path: pom.xml
##########
@@ -1974,6 +1974,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
<suppressionsLocation>hadoop-hdds/dev-support/checkstyle/suppressions.xml</suppressionsLocation>
<includeTestSourceDirectory>true</includeTestSourceDirectory>
<failOnViolation>false</failOnViolation>
+ <consoleOutput>true</consoleOutput>
Review comment:
I'd rather not add this. Errors are already printed by `checkstyle:check`, now they appear twice:
```
08:52:48,115 [INFO] --- maven-checkstyle-plugin:3.1.0:check (default-cli) @ hadoop-hdds-common ---
08:52:48,248 [INFO] Starting audit...
[ERROR] hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:52:17: 'static' modifier out of order with the JLS suggestions. [ModifierOrder]
[ERROR] hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:52:31: Name 'uuidString' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. [ConstantName]
[ERROR] hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:83: Line is longer than 80 characters (found 290). [LineLength]
Audit done.
08:52:48,250 [INFO] There are 3 errors reported by Checkstyle 8.29 with hadoop-hdds/dev-support/checkstyle/checkstyle.xml ruleset.
08:52:48,252 [ERROR] src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:[52,17] (modifier) ModifierOrder: 'static' modifier out of order with the JLS suggestions.
08:52:48,252 [ERROR] src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:[52,31] (naming) ConstantName: Name 'uuidString' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
08:52:48,253 [ERROR] src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:[83] (sizes) LineLength: Line is longer than 80 characters (found 290).
08:52:48,253 [WARNING] checkstyle:check violations detected but failOnViolation set to false
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org