Hi zookeepers, I have proceeded ZOOKEEPER-3446[1] 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. 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. [1] https://github.com/apache/zookeeper/pull/1006 On 2019/06/23 13:10:48, "Justin Ling Mao" <[email protected]> wrote: > 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.>
