zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zili Chen <wander4...@gmail.com>
Subject Re: [Request For Review] ZOOKEEPER-3474 a.k.a GH-1028
Date Tue, 06 Aug 2019 05:57:13 GMT
>I see some javadoc related issues in your gist,
>didn't we disable that rule ? JavadocType

The failures reported are "Missing javadoc", and
what we disabled is "Empty javadoc". Fair enough to
disable "Missing javadoc" for now and filed into
ZOOKEEPER-3469 since a empty javadoc is also a
placeholder.

>Are you using some automatic tool ?

Yes, IntelliJ helps a lot.
Best,
tison.


Enrico Olivelli <eolivelli@gmail.com> 于2019年8月6日周二 下午1:13写道:

> Il giorno mar 6 ago 2019 alle ore 03:51 Zili Chen <wander4096@gmail.com>
> ha
> scritto:
>
> > Hi Enrico,
> >
> > Thanks for your participant!
> >
> > Running after turn on checkstyle on zookeeper-server
> > it reports about 9000 checkstyle failures(see failures.txt[1])
> > among 596 files(see files.txt[1]). Most of them
> > are related to whitespace and import.
> >
>
> I see some javadoc related issues in your gist,
> didn't we disable that rule ? JavadocType
>
>
> > I think I can handle it with one or two whole days but
> > I'm afraid that I have no spare time until next weekend(08.17).
> >
>
> Are you using some automatic tool ?
> In the past few months in my company I have been working in adding
> checkstyle to other projects and
> we used Apache NetBeans to fix up the code, I guess IntelliJ can do the
> same.
>
> Enrico
>
>
> >
> > Also I would prefer make it into one big PR and separate
> > commits per file, i.e., resolving failures per file for
> > smoothly review.
> >
> > Best,
> > tison.
> >
> > [1] https://gist.github.com/TisonKun/ba69524defbf3e9d4c99eaf2de338e5a
> >
> >
> > Zili Chen <wander4096@gmail.com> 于2019年8月6日周二 上午9:43写道:
> >
> > > Hi Enrico,
> > >
> > > Thanks for your participant!
> > >
> > > Running after turn on checkstyle on zookeeper-server
> > > it reports about 9000 checkstyle failures(see out.txt)
> > > among 596 files(see conflicts.txt). Most of them
> > > are related to whitespace and import.
> > >
> > > I think I can handle it with one or two whole days but
> > > I'm afraid that I have no spare time until next weekend(08.17).
> > >
> > > Also I would prefer make it into one big PR and separate
> > > commits per file, i.e., resolving failures per file for
> > > smoothly review.
> > >
> > > Best,
> > > tison.
> > >
> > >
> > > Enrico Olivelli <eolivelli@gmail.com> 于2019年8月5日周一 下午9:37写道:
> > >
> > >> Il giorno lun 5 ago 2019 alle ore 14:03 Zili Chen <
> wander4096@gmail.com
> > >
> > >> ha
> > >> scritto:
> > >>
> > >> > Hi zookeepers,
> > >> >
> > >> > Recently I've sent a pr[1] on enable checkstyle configuration on
> > >> > zookeeper-promethus-metrics, Enrico(committer) and
> Dinesh(contributor)
> > >> > kindly reviewed it but we still need another look from committer as
> > >> > process required. Someone of you has spare time to have a look?
> > >> >
> > >>
> > >> Tison,
> > >> Norbert approved the PR and I have merged it.
> > >> We have already begun this check style work.
> > >> Thank you so much to you and Mao Ling for these efforts.
> > >>
> > >>
> > >> >
> > >> > Besides, as ZOOKEEPER-3431 described, we're going to enable this
> > >> > configuration on zookeeper-server as following. This is really a
> major
> > >> > package which contains a lot of codes involved by multi prs. I'd
> like
> > to
> > >> > hear from the community about how to proceed.
> > >> >
> > >>
> > >> I think that if we do it quickly there will be as few troubles as
> > >> possible.
> > >> I am available for review, just ping me.
> > >>
> > >>
> > >>
> > >> >
> > >> > (From where I stand, enable on the whole package at once
> > >> > could solve the problem at once, but it would conflict quite a lot
> > >> > ongoing prs. On the other hand, sending the pr without timely
> reviews
> > >> > also causes a burden to rebase it on the master and solve conflict
> > >> > again and again(since it likely conflicts with a merged pr))
> > >> >
> > >>
> > >> Did you check how many violations we have con zookeeper-server ?
> > >> How much big is the work ?
> > >> Do you have an estimate ?
> > >>
> > >> We could take into consideration to have only one big PR.
> > >> The tradeoff is that it will be very difficult to perform a review, we
> > >> will
> > >> need more eyes on the diff
> > >>
> > >> Enrico
> > >>
> > >>
> > >>
> > >>
> > >> >
> > >> > Best,
> > >> > tison.
> > >> >
> > >> > [1] https://github.com/apache/zookeeper/pull/1028
> > >> >
> > >>
> > >
> >
>

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