zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Enrico Olivelli <eolive...@gmail.com>
Subject Re: Clean up the all the checkstyle violations in the zookeeper-server module
Date Sun, 23 Jun 2019 17:26:41 GMT
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.

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