The FE java format check is now required. If you need to check it in local env, please refer to http://doris.incubator.apache.org/developer/developer-guide/java-format-code.html#import-order
-- 此致！Best Regards 陈明雨 Mingyu Chen Email: morning...@apache.org 在 2022-06-14 22:00:09，"陈明雨" <morning...@163.com> 写道： >I'm not sure if we currently have some overly restrictive rules that the >developer hard to follow. > >However, I think we can try to turn on the required check, observe the >recently submitted PR, and if any unreasonable checks are found, >modify them in time to prevent such checks from affecting the efficiency of >code merging. > > > > >-- > >此致！Best Regards >陈明雨 Mingyu Chen > >Email: >morning...@apache.org > > > > > >At 2022-06-14 21:36:26, "morrysnow" <morrys...@126.com> wrote: >>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 >>
