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 06:35:40 GMT
A solution could be, we remains current simple configuration
and introduce a so-called "strict-checkstyle.xml" and apply
it per package. Once we enable it on every package, we can
merge it into the simple configuration.

Best,
tison.


Enrico Olivelli <eolivelli@gmail.com> 于2019年6月29日周六 下午2:19写道:

> Il sab 29 giu 2019, 07:59 Zili Chen <wander4096@gmail.com> 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 <eolivelli@gmail.com> 于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 <nkalmar@cloudera.com.invalid
> >
> > 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 <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.
> > > > >
> > > >
> > >
> >
>

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