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 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@doris.apache.org For additional commands, e-mail: dev-h...@doris.apache.org
