Hi Andor, To be exact, "iterations" means we define the original rules in checkstyle configuration at once and turn them on one package after another, so iterations. Is it correct?
Best, tison. Andor Molnar <[email protected]> 于2019年7月1日周一 下午9:09写道： > 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. > >>>>>> > >>>>> > >>>> > >>> > >> > >
