Il mar 4 lug 2017, 18:06 Dávid Szigecsán <sige...@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 <guosi...@gmail.com>: > > > Those modules are fine, they are rarely touched any way. > > > > On Jul 4, 2017 8:57 AM, "Enrico Olivelli" <eolive...@gmail.com> wrote: > > > > > 2017-07-04 16:50 GMT+02:00 Sijie Guo <guosi...@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" <eolive...@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
