I like the idea of doing this in iterations. Andor
> On 2019. Jun 29., at 8:35, Zili Chen <[email protected]> wrote: > > A solution could be, we remains current simple configuration > and introduce a so-called "strict-checkstyle.xml" and apply > it per package. Once we enable it on every package, we can > merge it into the simple configuration. > > Best, > tison. > > > Enrico Olivelli <[email protected]> 于2019年6月29日周六 下午2:19写道： > >> Il sab 29 giu 2019, 07:59 Zili Chen <[email protected]> ha scritto: >> >>> Thank you Enrico. It seems my previous reply delivered >>> into another thread. Repost below. >>> >>> Hi zookeepers, >>> >>> I have proceeded ZOOKEEPER-3446 and been guided to here >>> to discuss for a consensus before any more efforts. >>> >>> In general， +1 on introducing and forcing checkstyle. >>> >>> About the process, I agree that we firstly reach a consensus >>> on the configuration and enable it per package. In order for >>> our contributors to rebase as few times as possible, we'd >>> better introduce all rules we all agree on at once. Note that >>> we could always add rule if someone ask for and agreed by the >>> community. >>> >>> Currently, we turn on checkstyle on all modules. >> >> >> This is quite important because we are using inlt in order to prevent >> @author tags. >> This was part of the ant based precommit job >> >> Enrico >> >> >> >> >> Following the >>> process above, we firstly turn off it once we apply the new >>> configuration, and then turn on it per package. >>> >>> If the community is willing to do this work, a JIRA about the new >>> checkstyle configuration should be filed and we continue the discussion >>> there. Generally, rules proposed by Enrico are good start point and >>> I agree on we should not introduce anything "fancy", but according to >>> what is actually needed. >>> >>> Best, >>> tison. >>> >>> >>> Enrico Olivelli <[email protected]> 于2019年6月29日周六 下午1:51写道： >>> >>>> @Zili Chen >>>> This is the original email thread. So you can answer here. >>>> >>>> Enrico >>>> >>>> Il ven 28 giu 2019, 11:04 Norbert Kalmar <[email protected] >>> >>> ha >>>> scritto: >>>> >>>>> Community is eager to jump on on this, we already have pull requests >>>>> cleaning up imports :) >>>>> >>>>> First of all, sorry for the late reply (I thought I already answered >>>> this, >>>>> I remember reading it and drawing up an answer. Oh well) >>>>> >>>>> Some big patches are already reviewed, I think we should commit as >> much >>>> as >>>>> possible before doing this refactor. (I'll also try to rev up my code >>>>> review/commit thread) >>>>> >>>>> As for waiting for 3.6.0 - I don't see the reason we should. Unless >> of >>>>> course this would delay the release too much... >>>>> >>>>> I haven't checked HBase checkstyle against our code, I don't think we >>>>> should introduce anything "fancy". What Enrico listed up sounds like >> a >>>> good >>>>> starting point. >>>>> >>>>> +1 on introducing and forcing checkstyle. >>>>> >>>>> Regards, >>>>> Norbert >>>>> >>>>> >>>>> On Sun, Jun 23, 2019 at 7:27 PM Enrico Olivelli <[email protected] >>> >>>>> wrote: >>>>> >>>>>> Justin, >>>>>> Thank you so much for your help in this. >>>>>> >>>>>> I would suggest to apply all of the rules in one pass, splitting >> the >>>> work >>>>>> per package. >>>>>> This way reviews will be easier, we will limit the number of >> commits >>>> and >>>>> we >>>>>> won't annoy too much the contributors , asking for hard rebases >>>>>> >>>>>> This is how we did it on Apache Bookkeeper >>>>>> https://github.com/apache/bookkeeper/issues/230 >>>>>> >>>>>> I will help review and commit all of your patches, it will be >>> mostly a >>>>>> matter of code reformat without any behavior change. >>>>>> Currently I am doing the same kind of work on others projects of my >>>>>> company, so I perfectly know how the work will go. >>>>>> >>>>>> Before starting we must ensure that: >>>>>> 1) community is willing to do this work (we will force a rebase on >>>> mostly >>>>>> every pending PR) >>>>>> 2) the proposed configuration is accepted by the community >>>>>> 3) it is the good time to do it, or should we wait for 3.6.0 to be >>> out >>>>>> >>>>>> >>>>>> I see you are referring to hbase checkstyle file, did you already >>>>> checked >>>>>> how much different it is from current project style? >>>>>> Will we only need to remove trailing spaces, reorder members, fix >>>>> imports, >>>>>> cut long lines ? >>>>>> >>>>>> >>>>>> Cheers >>>>>> Enrico >>>>>> >>>>>> >>>>>> >>>>>> Il dom 23 giu 2019, 15:11 Justin Ling Mao < >> [email protected] >>>> >>>> ha >>>>>> scritto: >>>>>> >>>>>>> Background:zookeeper-server is the main-module of the zk >> codebase. >>>>>>> Unfortunately, there were many checkstyle violations in it. To >>>> improve >>>>>> the >>>>>>> code quality and code standards, >>>>>>> IMHO, it's time to clean up the all the checkstyle >> violations(turn >>> on >>>>> the >>>>>>> <failOnViolation>true</failOnViolation>). we can learn from the >>> hbase >>>>>> whose >>>>>>> checkstyle(almost 40+ rules) is very strict and ensures a very >>>> unified >>>>>> code >>>>>>> style.( >>>>>>> >>>>>> >>>>> >>>> >>> >> https://github.com/apache/hbase/blob/master/hbase-checkstyle/src/main/resources/hbase/checkstyle.xml >>>>>>> ) >>>>>>> My planing is: clean up the all the checkstyle one rule by >> another >>> to >>>>>>> avoid too much code changes for review. >>>>>>> Everything's hard in the beginning, I have fired my first shot( >>>>>>> https://github.com/apache/zookeeper/pull/992). >>>>>>> If this draft has accepted by the community, I will create the >>>>>>> corresponding sub-tasks for more people joining this work. >>>>>>> >>>>>>> Cited the comment from Enrico Olivelli in the >>>>>>> >>>>>> >>>>> >>>> >>> >> ZOOKEEPER-3431:--------------------------------------------------------------we >>>>>>> have to discuss this topic with the community.I have experience >> on >>>>>>> BookKeeper, we had to clean up groups of packages.This is the >> kind >>> of >>>>>> stuff >>>>>>> that invalidates all of the pending pull requests.I have already >>> sett >>>>> up >>>>>> a >>>>>>> basic checkstyle configuration file and it is already active but >> it >>>> is >>>>>>> performing only very basic checks (like no 'author' tags).I will >>>>>> appreciate >>>>>>> very much if you want to drive this effort, personally I would >>> start >>>>> this >>>>>>> stuff after 3.6.0 release, once we consolidate current master and >>> the >>>>>> maven >>>>>>> build. I would have sent an email on the dev@ list soon.We also >>> have >>>>> to >>>>>>> agree on the checkstyle configuration, it is not trivial, I would >>>> take >>>>>> the >>>>>>> file from HBase, BookKeeper or other ASF projects on the Haddoop >>>>>> ecosystem. >>>>>> >>>>> >>>> >>> >>
