zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zili Chen <wander4...@gmail.com>
Subject Re: Clean up the all the checkstyle violations in the zookeeper-server module
Date Sat, 29 Jun 2019 04:00:32 GMT
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" <m...@sina.com> 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.>

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