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