Hi , all devs After this PR: https://github.com/apache/incubator-doris/pull/10134 <https://github.com/apache/incubator-doris/pull/10134>, all checkstyle error has been fixed. So, i want to turn Github workflow 'FE Code Style Checker' to 'Required' after this PR be merged.
After #10134 be merged, when compiling fe with `maven`, `CheckStyle` checks are done by default. This will slightly slow down compilation. If you want to skip checkstyle, please use the following command to compile mvn clean install -DskipTests -Dcheckstyle.skip Also, you can check your code style seperately by run following command under fe directory: mvn checkstyle:check > 2022年5月6日 16:16，ling miao <lingm...@apache.org> 写道： > > Regarding the code format correction of the old code, do we currently need > to do it manually? > > Ling Miao > > morrysnow <morrys...@126.com> 于2022年5月6日周五 15:03写道： > >> Hi devs, >> >> I create issues for every steps. Any one willing to submit pr could assign >> issue to himself/herself. >> >> Step1: https://github.com/apache/incubator-doris/issues/9403 < >> https://github.com/apache/incubator-doris/issues/9403> >> Step2: https://github.com/apache/incubator-doris/issues/9404 >> Step3: https://github.com/apache/incubator-doris/issues/9405 >> Step4: https://github.com/apache/incubator-doris/issues/9406 >> >>> 2022年4月29日 13:43，morrysnow <morrys...@126.com> 写道： >>> >>> Hi, devs >>> >>> I want to add Step 0 before all steps. >>> >>> Step 0: >>> >>> Add checkstyle action to github action to check pr style. This action >> checks incremental code and adds comment to pr automatically. >>> Action: https://github.com/dbelyaev/action-checkstyle < >> https://github.com/dbelyaev/action-checkstyle> >>> Demo: https://github.com/morrySnow/test_checkstyle/pull/5 < >> https://github.com/morrySnow/test_checkstyle/pull/5> >>> >>>> 2022年4月29日 13:40，morrysnow <morrys...@126.com> 写道： >>>> >>>> Hi, mingyu >>>> >>>> 1. Check license is needed for local check. >>>> 2. Add a rule to forbidden static import is ok for me >>>> >>>>> 2022年4月28日 21:34，陈明雨 <morning...@163.com> 写道： >>>>> >>>>> That would be great! >>>>> Some suggestion about the rules: >>>>> >>>>> >>>>>> d. license header check >>>>> For now, we use skywalking-eyes to check license header, is this rule >> still needed? >>>>> >>>>> >>>>>> adjust the order of import to "static, org.apache.doris, third party, >> java" >>>>> Better not using static import >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> >>>>> 此致！Best Regards >>>>> 陈明雨 Mingyu Chen >>>>> >>>>> Email: >>>>> chenmin...@apache.org >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> 在 2022-04-28 17:14:34，"morrysnow" <morrys...@126.com> 写道： >>>>>> Hi, devs, >>>>>> >>>>>> I split adding java check style into 5 steps. I’m willing to get >> feedback about it. >>>>>> >>>>>> CheckStyle and Spotless will modify about 10k lines. >>>>>> >>>>>> Step 1: >>>>>> >>>>>> 1. Add Spotless plugin and do some simple format(1198 files changed, >> 3205 insertions(+), 4350 deletions(-)) >>>>>> a. end with new line >>>>>> b. trim trailing whitespace >>>>>> c. replace tab indent with 4 spaces indent >>>>>> d. license header check >>>>>> e. adjust the order of import to "static, org.apache.doris, third >> party, java" >>>>>> f. remove unused imports >>>>>> g. make sure all files are encoding by utf-8 >>>>>> h. make sure all files line ending by LF >>>>>> >>>>>> 2. Add checkstyle rules but set default severity to warning to avoid >> compile failed. >>>>>> >>>>>> 3. set some rules' severity to error in Checkstyle since no lines in >> doris break these rules(0 files, 0 lines) >>>>>> a. Merge conflicts unresolved >>>>>> b. Avoid using corresponding octal or Unicode escape >>>>>> c. Avoid Escaped Unicode Characters >>>>>> d. No Line Wrap >>>>>> e. Package Name >>>>>> f. Type Name >>>>>> g. Annotation Location >>>>>> h. Interface Type Parameter >>>>>> i. CatchParameterName >>>>>> j. Pattern Variable Name >>>>>> k. Record Component Name >>>>>> l. Record Type Parameter Name >>>>>> m. Method Type Parameter Name >>>>>> >>>>>> 4. Set below rules severity to error in Checkstyle since these had >> done by splotless(12 files, 177 lines) >>>>>> a. Redundant Import >>>>>> b. Custom Import Order >>>>>> c. Unused Imports >>>>>> d. Avoid Star Import >>>>>> e. tab character in file >>>>>> f. Newline At End Of File >>>>>> g. Trailing whitespace found >>>>>> >>>>>> >>>>>> >>>>>> Step 2: (241 files, 962 lines (with Step 1)) >>>>>> >>>>>> 1. Set below rules' severity to error in Checkstyle, all rules are >> import to make sure the code is correct >>>>>> a. Need Braces >>>>>> b. Equals Hash Code >>>>>> c. Missing Switch Default >>>>>> d. Fall Through >>>>>> e. No Finalizer >>>>>> >>>>>> >>>>>> 2. Set below rules' severity to error in Checkstyle, all rules are >> about name >>>>>> a. Member Name >>>>>> b. Constant Name >>>>>> c. Parameter Name >>>>>> d. LambdaParameterName >>>>>> e. Local Variable Name >>>>>> f. Class Type Parameter Name >>>>>> g. Static Variable Name >>>>>> h. Method Name >>>>>> j. Abbreviation As Word In Name >>>>>> >>>>>> >>>>>> Step 3: (605 files, 6008 lines (with Step 1 and Step 2)) >>>>>> >>>>>> 1. Set below rules' severity to error in Checkstyle, all rules are >> about whitespace >>>>>> a. Empty Block >>>>>> b. Left Curly >>>>>> c. Right Curly >>>>>> d. White space Around >>>>>> e. Generic Whitespace >>>>>> f. Indentation >>>>>> g. No Whitespace Before Case Default Colon >>>>>> h. Method Param Pad >>>>>> i. Whitespace >>>>>> j. Paren Pad >>>>>> k. Extra newline >>>>>> l. Extra newline at end of block >>>>>> m. Empty Statement >>>>>> n. Empty Catch Block >>>>>> o. Comments Indentation >>>>>> >>>>>> 2. Set below rules' severity to error in Checkstyle, all rules are >> about the position of the newline >>>>>> a. Line Length 120 >>>>>> b. Operator Wrap >>>>>> c. One Statement Per Line >>>>>> d. Multiple Variable Declarations >>>>>> >>>>>> >>>>>> Step 4: (674 files, 6730 lines (with Step 1, Step 2 and Step 3)) >>>>>> >>>>>> 1. Set below rules' severity to error in Checkstyle >>>>>> a. Array Type Style >>>>>> b. Upper Ell >>>>>> c. Modifier Order >>>>>> d. Empty Line Separator >>>>>> e. Separator Wrap >>>>>> f. Overload Methods Declaration Order >>>>>> g. Variable Declaration Usage Distance >>>>>> i. One Top Level Class >>>>>> >>>>>> >>>>>> Step 5: >>>>>> >>>>>> 1. Add java doc check to CheckStyle and set severity to warning. >>>>>> >>>>>> 2. Fix java doc warning gradually >>>>>> >>>>>> >>>>>>> 2022年4月28日 00:39，morrysnow <morrys...@126.com> 写道： >>>>>>> >>>>>>> Hi, devs >>>>>>> >>>>>>> I summarized all the rules I wanted to add. I examined the number of >> lines of code affected by all the rules and categorized them by importance. >>>>>>> >>>>>>> The link is here: >> https://github.com/apache/incubator-doris/issues/8985#issuecomment-1111214162 >> < >> https://github.com/apache/incubator-doris/issues/8985#issuecomment-1111214162 >>> >>>>>>> >>>>>>> I would like to be able to submit them in multiple patches. The >> principle of patch splitting is as follows. Those with low impact and high >> importance are submitted first and sets the severity to Error. The Javadoc >> commits last and sets the severity to Warning. >>>>>>> >>>>>>> >>>>>>>> 2022年4月18日 16:09，vin jake <jakevin...@gmail.com> 写道： >>>>>>>> >>>>>>>> Hi, there is a new PR about format doris. >>>>>>>> https://github.com/apache/incubator-doris/pull/9072 >>>>>>>> >>>>>>>> I think it's time to ensure what checkstyle rules we should add >> for fe. >>>>>>>> >>>>>>>> All people who care about format rule can put up your thoughts. >>>>>>>> >>>>>>>> We can add it in the issue: >>>>>>>> https://github.com/apache/incubator-doris/issues/8985 >>>>>>>> >>>>>>>> >>>>>>>> On Thu, Apr 14, 2022 at 6:52 PM vin jake <jakevin...@gmail.com> >> wrote: >>>>>>>> >>>>>>>>> great, I will trim it in my PR. >>>>>>>>> >>>>>>>>> checkstyle.xml need more discussion >>>>>>>>> >>>>>>>>> On Thu, Apr 14, 2022 at 5:48 PM morrysnow <morrys...@126.com> >> wrote: >>>>>>>>> >>>>>>>>>> I agree with u. >>>>>>>>>> >>>>>>>>>> For the first point, I want to list rules first, and then change >>>>>>>>>> checkstyle.xml. >>>>>>>>>> >>>>>>>>>> For the second point, original change information in git is not >> lost, we >>>>>>>>>> just need to do blame on the version prior to the ‘code style’ >> commit and >>>>>>>>>> some gui tools could list history of one file. Anyway, it is not >> very >>>>>>>>>> convenience. >>>>>>>>>> >>>>>>>>>>> 2022年4月14日 17:35，Shuo Wang <wangshuo...@gmail.com> 写道： >>>>>>>>>>> >>>>>>>>>>> In general, I believe that we should do some work to make the >> code clean >>>>>>>>>>> and readable. >>>>>>>>>>> >>>>>>>>>>> My concern is: >>>>>>>>>>> 1. We should have an agreement on the code style specification >> in the >>>>>>>>>>> community at first. >>>>>>>>>>> 2. If the many lines of code change after applying the code >> style rule, >>>>>>>>>> we >>>>>>>>>>> would lose the original changelog via `git blame`. >>>>>>>>>>> >>>>>>>>>>> vin jake <jakevin...@gmail.com> 于2022年4月14日周四 17:12写道： >>>>>>>>>>> >>>>>>>>>>>> I have add it in PR >>>>>>>>>> https://github.com/apache/incubator-doris/pull/8987 >>>>>>>>>>>> >>>>>>>>>>>> On Thu, Apr 14, 2022 at 4:37 PM morrysnow <morrys...@126.com> >> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi, devs, >>>>>>>>>>>>> >>>>>>>>>>>>> Currently, we only have two rules in checkstyle.xml in fe. >> These are >>>>>>>>>> all >>>>>>>>>>>>> about import. So, the code style in fe is very casual. >>>>>>>>>>>>> I want to add more rules to checkstyle.xml in fe to Improve >> code >>>>>>>>>>>>> readability, and adjust all fe code to satisfy new code style >> step by >>>>>>>>>>>> step. >>>>>>>>>>>>> What do you think about it? If this is a good idea. I will >> research >>>>>>>>>> which >>>>>>>>>>>>> rules apply to our code and put together a list. >>>>>>>>>>>>> >>>>>>>>>>>>> >> --------------------------------------------------------------------- >>>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@doris.apache.org >>>>>>>>>>>>> For additional commands, e-mail: dev-h...@doris.apache.org >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >> --------------------------------------------------------------------- >>>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@doris.apache.org >>>>>>>>>> For additional commands, e-mail: dev-h...@doris.apache.org >>>>>>>>>> >>>>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: dev-unsubscr...@doris.apache.org >>>>>> For additional commands, e-mail: dev-h...@doris.apache.org >>> >> >> > > -- > Ling Miao | Apache Doris
