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
