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
