You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/05/08 03:04:10 UTC
[GitHub] [orc] williamhyun opened a new pull request #697: ORC-795: Add LineLength checkstyle rule
williamhyun opened a new pull request #697:
URL: https://github.com/apache/orc/pull/697
### What changes were proposed in this pull request?
This PR aims to add the `LineLength` checkstyle rule.
### Why are the changes needed?
There is no rule restricting line length, it would be great if we have this.
### How was this patch tested?
Pass the CIs.
--
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
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #697: ORC-795: Add LineLength checkstyle rule
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #697:
URL: https://github.com/apache/orc/pull/697#discussion_r628792833
##########
File path: java/pom.xml
##########
@@ -279,6 +279,9 @@
</module>
<module name="NewlineAtEndOfFile">
</module>
+ <module name="LineLength">
+ <property name="max" value="125"/>
Review comment:
Thank you, @williamhyun . Got it. I checked with `<property name="max" value="100"/>` and `<failOnViolation>false</failOnViolation>` and realized that we have too many instances although this PR fixes 125+ length. We had better reduce this gradually to 100 or 80 later.
```
$ mvn checkstyle:check -Panalyze | grep ERROR | wc -l
Using `mvn` from path: /usr/local/bin/mvn
69
```
--
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
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #697: ORC-795: Add LineLength checkstyle rule
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #697:
URL: https://github.com/apache/orc/pull/697#discussion_r628792833
##########
File path: java/pom.xml
##########
@@ -279,6 +279,9 @@
</module>
<module name="NewlineAtEndOfFile">
</module>
+ <module name="LineLength">
+ <property name="max" value="125"/>
Review comment:
Thank you, @williamhyun . Got it. I checked with `<property name="max" value="100"/>` and `<failOnViolation>false</failOnViolation>` and realized that we have too many instances after this PR. We had better reduce this gradually to 100 or 80 later.
```
$ mvn checkstyle:check -Panalyze | grep ERROR | wc -l
Using `mvn` from path: /usr/local/bin/mvn
69
```
--
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
[GitHub] [orc] dongjoon-hyun commented on pull request #697: ORC-795: Add LineLength checkstyle rule
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #697:
URL: https://github.com/apache/orc/pull/697#issuecomment-838994292
Merged to main/1.6.
--
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
[GitHub] [orc] dongjoon-hyun merged pull request #697: ORC-795: Add LineLength checkstyle rule
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun merged pull request #697:
URL: https://github.com/apache/orc/pull/697
--
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
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #697: ORC-795: Add LineLength checkstyle rule
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #697:
URL: https://github.com/apache/orc/pull/697#discussion_r628792833
##########
File path: java/pom.xml
##########
@@ -279,6 +279,9 @@
</module>
<module name="NewlineAtEndOfFile">
</module>
+ <module name="LineLength">
+ <property name="max" value="125"/>
Review comment:
Thank you, @williamhyun . Got it. I checked with `<property name="max" value="100"/>` and `<failOnViolation>false</failOnViolation>` and realized that we have too many instances although this PR fixes 125+ length. We had better reduce this gradually to 100 or 80 later after merging this PR.
```
$ mvn checkstyle:check -Panalyze | grep ERROR | wc -l
Using `mvn` from path: /usr/local/bin/mvn
69
```
--
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
[GitHub] [orc] dongjoon-hyun commented on pull request #697: ORC-795: Add LineLength checkstyle rule
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #697:
URL: https://github.com/apache/orc/pull/697#issuecomment-838973267
Let me merge this, @williamhyun ~
--
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