Hi , all devs
After this PR: 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 At 2022-04-29 13:43:18, "morrysnow" <morrys...@126.com> wrote: >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 >
