zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Norbert Kalmar <nkal...@cloudera.com.INVALID>
Subject Re: Clean up the all the checkstyle violations in the zookeeper-server module
Date Fri, 28 Jun 2019 09:03:38 GMT
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.


On Sun, Jun 23, 2019 at 7:27 PM Enrico Olivelli <eolivelli@gmail.com> 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 <maoling199210191@sina.com> 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.

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message