John, I saw David updated the issue https://github.com/apache/bookkeeper/issues/230 with list of the packages to enable checkstyle.
Feel free to coordinate with David to pick the packages to work on :) - Sijie On Tue, Jul 4, 2017 at 12:46 PM, John Lonergan <john.loner...@gmail.com> wrote: > Re"It contains almost 5000 issues." > > Have you put the gist somewhere for information. > Folk like myself are probably happy to contribute some time. It's an easy > way to contribute something to the community. > > JL > > On 4 Jul 2017 5:17 pm, "Enrico Olivelli" <eolive...@gmail.com> wrote: > >> >> >> 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 >> >
