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