You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/18 15:40:52 UTC
[GitHub] [incubator-doris] jackwener opened a new pull request, #9089: [enhancement](*): add checkstyle file
jackwener opened a new pull request, #9089:
URL: https://github.com/apache/incubator-doris/pull/9089
# Proposed changes
Issue Number: related #8985
## Problem Summary:
add checkstyle file
## Checklist(Required)
1. Does it affect the original behavior: (Yes)
2. Has unit tests been added: (No Need)
3. Has document been added or modified: (No Need)
4. Does it need to update dependencies: (No)
5. Are there any changes that cannot be rolled back: (No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] EmmyMiao87 commented on a diff in pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on code in PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#discussion_r853686729
##########
fe/checkstyle.xml:
##########
@@ -26,8 +26,173 @@
<property name="fileExtensions" value="java"/>
</module>
<module name="SuppressWarningsFilter"/>
+ <module name="FileTabCharacter"/>
+ <module name="NewlineAtEndOfFile">
+ <property name="lineSeparator" value="lf"/>
+ </module>
+ <property name="charset" value="UTF-8"/>
+ <!-- Checks if a line is too long. -->
+ <module name="LineLength">
+ <property name="max" value="150"/>
+ <property name="severity" value="error"/>
+ <property name="ignorePattern" value="^(package .*;\s*)|(import .*;\s*)|( *\* .*https?://.*)$"/>
+ </module>
+
<module name="TreeWalker">
- <module name="AvoidStarImport"/>
- <module name="UnusedImports"/>
+ <module name="PackageDeclaration"/>
+ <module name="UnnecessaryParentheses"/>
+ <module name="SuppressWarningsHolder"/>
+ <module name="EmptyBlock">
+ <property name="option" value="text"/>
+ <property name="tokens" value="
+ LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_IF,
+ LITERAL_FOR, LITERAL_TRY, LITERAL_WHILE, INSTANCE_INIT, STATIC_INIT"/>
+ </module>
+ <module name="EmptyStatement"/>
+ <module name="EmptyForInitializerPad"/>
+ <module name="EmptyForIteratorPad">
+ <property name="option" value="space"/>
+ </module>
+ <module name="MethodParamPad">
+ <property name="allowLineBreaks" value="true"/>
+ <property name="option" value="nospace"/>
+ </module>
+ <module name="ParenPad"/>
+ <module name="TypecastParenPad"/>
+
+ <!-- Checks for braces around if and else blocks -->
+ <module name="NeedBraces">
+ <property name="severity" value="error"/>
+ <property name="tokens" value="LITERAL_IF, LITERAL_ELSE, LITERAL_FOR, LITERAL_WHILE, LITERAL_DO"/>
+ </module>
+
+ <!-- check { and } -->
+ <module name="LeftCurly">
+ <property name="tokens" value="ANNOTATION_DEF, CLASS_DEF, CTOR_DEF, ENUM_CONSTANT_DEF, ENUM_DEF,
+ INTERFACE_DEF, LAMBDA, LITERAL_CASE, LITERAL_CATCH, LITERAL_DEFAULT,
+ LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF,
+ LITERAL_SWITCH, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE, METHOD_DEF,
+ OBJBLOCK, STATIC_INIT, RECORD_DEF, COMPACT_CTOR_DEF"/>
+ </module>
+ <module name="RightCurly">
+ <property name="option" value="same"/>
+ <property name="severity" value="error"/>
+ </module>
+
+ <module name="GenericWhitespace"/>
+ <module name="WhitespaceAfter"/>
+ <module name="NoWhitespaceAfter"/>
+ <module name="NoWhitespaceBefore"/>
+ <module name="SingleSpaceSeparator"/>
+ <module name="Indentation">
+ <property name="throwsIndent" value="8"/>
+ <property name="lineWrappingIndentation" value="8"/>
+ </module>
+
+ <module name="UpperEll"/>
+ <module name="DefaultComesLast"/>
+ <module name="ArrayTypeStyle"/>
+ <module name="MultipleVariableDeclarations"/>
+ <module name="ModifierOrder"/>
+ <module name="OneStatementPerLine"/>
+ <module name="StringLiteralEquality"/>
+ <module name="MutableException"/>
+ <module name="EqualsHashCode"/>
+ <module name="InnerAssignment"/>
+ <module name="InterfaceIsType"/>
+ <module name="HideUtilityClassConstructor"/>
+ <module name="ExplicitInitialization"/>
+ <module name="OneTopLevelClass"/>
+
+ <!-- name check -->
+ <module name="TypeNameCheck">
+ <metadata name="altname" value="TypeName"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="ConstantNameCheck">
+ <metadata name="altname" value="ConstantName"/>
+ <property name="applyToPublic" value="true"/>
+ <property name="applyToProtected" value="true"/>
+ <property name="applyToPackage" value="true"/>
+ <property name="applyToPrivate" value="false"/>
+ <property name="format" value="^([A-Z][A-Za-z0-9_]*|FLAG_.*)$"/>
+ <message key="name.invalidPattern"
+ value="Variable ''{0}'' should be in ALL_CAPS (if it is a constant) or be private (otherwise)."/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="StaticVariableNameCheck">
+ <metadata name="altname" value="StaticVariableName"/>
+ <property name="applyToPublic" value="true"/>
+ <property name="applyToProtected" value="true"/>
+ <property name="applyToPackage" value="true"/>
+ <property name="applyToPrivate" value="true"/>
+ <property name="format" value="^[a-z][a-zA-Z0-9]*_?$"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="MemberNameCheck">
+ <metadata name="altname" value="MemberName"/>
+ <property name="applyToPublic" value="true"/>
+ <property name="applyToProtected" value="true"/>
+ <property name="applyToPackage" value="true"/>
+ <property name="applyToPrivate" value="true"/>
+ <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="MethodNameCheck">
+ <metadata name="altname" value="MethodName"/>
+ <property name="format" value="(^[a-z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$|Void)"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="ParameterName">
+ <property name="severity" value="error"/>
+ </module>
+ <module name="LocalFinalVariableName">
+ <property name="severity" value="error"/>
+ </module>
+ <module name="LocalVariableName">
+ <property name="severity" value="error"/>
+ </module>
+
+ <!-- import -->
+ <module name="AvoidStarImport">
+ <property name="severity" value="error"/>
+ </module>
+ <module name="RedundantImport">
+ <property name="severity" value="error"/>
+ <message key="import.redundancy"
+ value="Redundant import {0}."/>
+ </module>
+ <module name="UnusedImports">
+ <property name="severity" value="error"/>
+ <message key="import.unused"
+ value="Unused import: {0}."/>
+ </module>
+ <module name="ImportOrder">
+ <property name="groups" value="org.apache.doris,org.apache,*,javax,java"/>
Review Comment:
The import order should be like this
```
4=javax
3=java
2=org
1=com
0=org.apache.doris
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] jackwener commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1106348001
@cody195 Your consideration is right.
I made a draft #9113, I found there are some problem need to consider.
We should continue pushing this job in #8985.
If you'd like take part in it, you can care it in #8985
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cody195 commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
cody195 commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1106047553
> LGTM
Hi @EmmyMiao87, it is not a good idea to add `checkstyle` file with so many rules directly. The project already build failed, why you approved these changes?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] jackwener closed pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
jackwener closed pull request #9089: [enhancement](*): add checkstyle file
URL: https://github.com/apache/incubator-doris/pull/9089
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] jackwener commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1101513069
PTAL @EmmyMiao87 @morrySnow @wangshuo128
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1103593801
PR approved by at least one committer and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1103593847
PR approved by anyone and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] EmmyMiao87 commented on a diff in pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on code in PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#discussion_r853815804
##########
fe/checkstyle.xml:
##########
@@ -26,8 +26,196 @@
<property name="fileExtensions" value="java"/>
</module>
<module name="SuppressWarningsFilter"/>
+ <module name="FileTabCharacter"/>
+ <module name="NewlineAtEndOfFile">
+ <property name="lineSeparator" value="lf"/>
+ </module>
+ <property name="charset" value="UTF-8"/>
+ <!-- Checks if a line is too long. -->
+ <module name="LineLength">
+ <property name="max" value="120"/>
+ <property name="severity" value="error"/>
+ <property name="ignorePattern" value="^(package .*;\s*)|(import .*;\s*)|( *\* .*https?://.*)$"/>
+ </module>
+
<module name="TreeWalker">
- <module name="AvoidStarImport"/>
- <module name="UnusedImports"/>
+ <module name="PackageDeclaration"/>
+ <module name="EmptyBlock">
+ <property name="option" value="text"/>
+ <property name="tokens" value="
+ LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_IF,
+ LITERAL_FOR, LITERAL_TRY, LITERAL_WHILE, INSTANCE_INIT, STATIC_INIT"/>
+ </module>
+ <module name="EmptyStatement"/>
+ <module name="EmptyForInitializerPad"/>
+ <module name="EmptyForIteratorPad">
+ <property name="option" value="space"/>
+ </module>
+ <module name="MethodParamPad">
+ <property name="allowLineBreaks" value="true"/>
+ <property name="option" value="nospace"/>
+ </module>
+ <module name="ParenPad"/>
+ <module name="TypecastParenPad"/>
+
+ <!-- Checks for braces around if and else blocks -->
+ <module name="NeedBraces">
+ <property name="severity" value="error"/>
+ <property name="tokens" value="LITERAL_IF, LITERAL_ELSE, LITERAL_FOR, LITERAL_WHILE, LITERAL_DO"/>
+ </module>
+
+ <!-- check { and } -->
+ <module name="LeftCurly">
+ <property name="tokens" value="ANNOTATION_DEF, CLASS_DEF, CTOR_DEF, ENUM_CONSTANT_DEF, ENUM_DEF,
+ INTERFACE_DEF, LAMBDA, LITERAL_CASE, LITERAL_CATCH, LITERAL_DEFAULT,
+ LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF,
+ LITERAL_SWITCH, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE, METHOD_DEF,
+ OBJBLOCK, STATIC_INIT, RECORD_DEF, COMPACT_CTOR_DEF"/>
+ </module>
+ <module name="RightCurly">
+ <property name="option" value="same"/>
+ <property name="severity" value="error"/>
+ </module>
+
+ <module name="UpperEll"/>
+ <module name="DefaultComesLast"/>
+ <module name="ArrayTypeStyle"/>
+ <module name="MultipleVariableDeclarations"/>
+ <module name="ModifierOrder"/>
+ <module name="OneStatementPerLine"/>
+ <module name="OneTopLevelClass"/>
+
+ <module name="EqualsHashCode"/>
+ <module name="UnnecessaryParentheses"/>
+ <module name="SimplifyBooleanReturn"/>
+ <module name="SimplifyBooleanExpression"/>
+
+ <!-- name check -->
+ <module name="TypeNameCheck">
+ <metadata name="altname" value="TypeName"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="ConstantNameCheck">
+ <metadata name="altname" value="ConstantName"/>
+ <property name="applyToPublic" value="true"/>
+ <property name="applyToProtected" value="true"/>
+ <property name="applyToPackage" value="true"/>
+ <property name="applyToPrivate" value="false"/>
+ <property name="format" value="^([A-Z][A-Za-z0-9_]*|FLAG_.*)$"/>
+ <message key="name.invalidPattern"
+ value="Variable ''{0}'' should be in ALL_CAPS (if it is a constant) or be private (otherwise)."/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="StaticVariableNameCheck">
+ <metadata name="altname" value="StaticVariableName"/>
+ <property name="applyToPublic" value="true"/>
+ <property name="applyToProtected" value="true"/>
+ <property name="applyToPackage" value="true"/>
+ <property name="applyToPrivate" value="true"/>
+ <property name="format" value="^[a-z][a-zA-Z0-9]*_?$"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="MemberNameCheck">
+ <metadata name="altname" value="MemberName"/>
+ <property name="applyToPublic" value="true"/>
+ <property name="applyToProtected" value="true"/>
+ <property name="applyToPackage" value="true"/>
+ <property name="applyToPrivate" value="true"/>
+ <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="MethodNameCheck">
+ <metadata name="altname" value="MethodName"/>
+ <property name="format" value="(^[a-z][a-zA-Z0-9]*(_[a-zA-Z0-9]+)*$|Void)"/>
+ <property name="severity" value="error"/>
+ </module>
+ <module name="ParameterName">
+ <property name="severity" value="error"/>
+ </module>
+ <module name="LocalFinalVariableName">
+ <property name="severity" value="error"/>
+ </module>
+ <module name="LocalVariableName">
+ <property name="severity" value="error"/>
+ </module>
+
+ <!-- import -->
+ <module name="AvoidStarImport">
+ <property name="severity" value="error"/>
+ </module>
+ <module name="RedundantImport">
+ <property name="severity" value="error"/>
+ <message key="import.redundancy"
+ value="Redundant import {0}."/>
+ </module>
+ <module name="UnusedImports">
+ <property name="severity" value="error"/>
+ <message key="import.unused"
+ value="Unused import: {0}."/>
+ </module>
+ <module name="ImportOrder">
+ <property name="groups" value="org.apache.doris,com,org,java,*"/>
Review Comment:
```suggestion
<property name="groups" value="org.apache.doris,com.google.common,org,java,*"/>
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] jackwener commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1101510990
It's a base file. We can add more rule for it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cody195 commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
cody195 commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1101894409
Hi @jackwener, it is not a good idea to add `checkstyle` file with so many rules directly. The project will always build failed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] jackwener commented on pull request #9089: [enhancement](*): add checkstyle file
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9089:
URL: https://github.com/apache/incubator-doris/pull/9089#issuecomment-1101919343
Yes, it,s just for review.
Charles ***@***.***> 于 2022年4月19日周二 09:05写道：
> Hi @jackwener <https://github.com/jackwener>, it is not a good idea to
> add checkstyle file with so many rules directly. The project will always
> build failed.
>
> [image: image]
> <https://user-images.githubusercontent.com/86483005/163899937-60582d3f-81a5-45a6-840b-0de7c63a4bf8.png>
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-doris/pull/9089#issuecomment-1101894409>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHI4SLMJ7SZPU5BRXI4ORPLVFYBD3ANCNFSM5TWFDKWA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org