Re: Add more rules to checkstyle.xml in fe

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 merge

Re: Add more rules to checkstyle.xml in fe

Regarding the code format correction of the old code, do we currently need to do it manually? Ling Miao morrysnow 于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/incu

Re: Add more rules to checkstyle.xml in fe

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 Step2: https://github.com/apache/incubator-doris/issues/9404 Step3:

Re: Add more rules to checkstyle.xml in fe

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 Demo

Re: Add more rules to checkstyle.xml in fe

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，陈明雨 写道： > > 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

Re: Add more rules to checkstyle.xml in fe

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

Re: Add more rules to checkstyle.xml in fe

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-214162

Re: Add more rules to checkstyle.xml in fe

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-d

Re: Add more rules to checkstyle.xml in fe

great, I will trim it in my PR. checkstyle.xml need more discussion On Thu, Apr 14, 2022 at 5:48 PM morrysnow 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,

Re: Add more rules to checkstyle.xml in fe

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. Any

Re: Add more rules to checkstyle.xml in fe

very good -- Original -- From: morrysnow https://github.com/apache/incubator-doris/pull/8987 > > On Thu, Apr 14, 2022 at 4:37 PM morrysnow

Re: Add more rules to checkstyle.xml in fe

Great! But I think maybe we should list all rules in a some place and checked by every one care about it before we merge it to ensure the new code style is good enough. > 2022年4月14日 17:09，vin jake 写道： > > I have add it in PR https://github.com/apache/incubator-doris/pull/8987 > > On Thu, Apr

Re: Add more rules to checkstyle.xml in fe

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 changel

Re: Add more rules to checkstyle.xml in fe

I don’t know what exactly mean in ‘compile time’. Maven indeed to it with Checkstyle.xml in when compile. but we only check 'import with star' and 'unused import’ in fe now. We could do more than that such as import order, tabs and intents, comments style and etc.. We can provide code style scheme

Re: Add more rules to checkstyle.xml in fe

> I have add it in PR https://github.com/apache/incubator-doris/pull/8987 Good job! On Thu, Apr 14, 2022 at 5:12 PM vin jake wrote: > I have add it in PR https://github.com/apache/incubator-doris/pull/8987 > > On Thu, Apr 14, 2022 at 4:37 PM morrysnow wrote: > > > Hi, devs, > > > > Currently,

Re: Add more rules to checkstyle.xml in fe

I have add it in PR https://github.com/apache/incubator-doris/pull/8987 On Thu, Apr 14, 2022 at 4:37 PM morrysnow 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

Re: Add more rules to checkstyle.xml in fe

Is there a way to check the imports order? Now we can only manually remind the order by reviewers. Jianliang Qi On Thu, Apr 14, 2022 at 5:02 PM ling miao wrote: > It is best to check the code style at compile time. > > Ling Miao > > morrysnow 于2022年4月14日周四 16:37写道： > > > Hi, devs, > > > > Curr

Re: Add more rules to checkstyle.xml in fe

It is best to check the code style at compile time. Ling Miao morrysnow 于2022年4月14日周四 16:37写道： > 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