bookkeeper-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Enrico Olivelli <eolive...@gmail.com>
Subject Re: BookKeeper and CheckStyle
Date Tue, 04 Jul 2017 16:17:01 GMT
Il mar 4 lug 2017, 18:06 Dávid Szigecsán <sigee15@gmail.com> ha scritto:

> Hi,
>
> Yes, I created a PR for the first part of the change. (All modules except
> bookkeeper-server)
> I started to do the second part (bookkeeper-server), but It is a huge
> change. It contains almost 5000 issues.
> I'm thinking about how to slice it up to small steps.
>

Thank you David,
Yep, the idea is to create a single patch per package if possible

Enrico


> 2017-07-04 17:06 GMT+02:00 Sijie Guo <guosijie@gmail.com>:
>
> > Those modules are fine, they are rarely touched any way.
> >
> > On Jul 4, 2017 8:57 AM, "Enrico Olivelli" <eolivelli@gmail.com> wrote:
> >
> > > 2017-07-04 16:50 GMT+02:00 Sijie Guo <guosijie@gmail.com>:
> > > > It is fine to me if we do modules by modules and packages by packages
> > in
> > > > bookkeeper-server. We can keep the changes smaller for reviews and
> > easier
> > > > to merge.
> > >
> > > I see in the issue and PR
> > > https://github.com/apache/bookkeeper/pull/231 that he is adding CS to
> > > every maven module except from bookkeeper-server
> > > maybe it is a good starting point.
> > > I have written a comment in order to invite him to join the list
> > >
> > > I am also OK with applying such changes to bookkeeper-server one
> > > package at a time
> > >
> > > -- Enrico
> > >
> > > >
> > > > Also, it might be good to also discuss on the issue to keep David
> > updated
> > > > if he is not in the dev@ list.
> > > >
> > > > Sijie
> > > >
> > > > On Jul 4, 2017 6:43 AM, "Enrico Olivelli" <eolivelli@gmail.com>
> wrote:
> > > >
> > > > Hi all,
> > > > as you can see from github emails there is an ongoing proposal to add
> > > > "checkstyle" plugin to BookKeeper build.
> > > > I am really in favour of this change. It is already used in
> > > > DistributedLog and it will ease the review, preventing us from
> writing
> > > > comments for minor typos.
> > > >
> > > > https://github.com/apache/bookkeeper/issues/230
> > > > https://github.com/apache/bookkeeper/issues/230
> > > >
> > > > Thanks to David (I hope he is subscribed to this list) we will be
> able
> > > > to add this kind of support soon.
> > > >
> > > > My concern is that this change will make us change all big pull
> > requests.
> > > >
> > > > We should decide when to get checkstyle in:
> > > > 1) as soon as possible (after review of the patch)
> > > > 2) before 4.5 release, as last step
> > > > 3) after merging biggest changes (Twitter changes and Salesforce
> > > > changes) which are waiting for review/merge
> > > > 4) defer to the start of 4.6
> > > >
> > > > My proposal is to defer to the start of 4.6, the only problem is that
> > > > David will be doing a big effort to keep the patch in synch with the
> > > > actual master
> > > >
> > > > -- Enrico
> > >
> >
>
-- 


-- Enrico Olivelli

Mime
View raw message