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