You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@bookkeeper.apache.org by Enrico Olivelli <eo...@gmail.com> on 2017/07/04 16:17:01 UTC

Re: BookKeeper and CheckStyle

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

Re: BookKeeper and CheckStyle

Posted by Sijie Guo <gu...@gmail.com>.
Awesome! Glad to see the community coordinate the efforts working on this.

Sijie

On Jul 5, 2017 5:48 AM, "Enrico Olivelli" <eo...@gmail.com> wrote:

> David,,
> I see you have created an issue for the first part of bookkeeper-server.
> Maybe you can also create issues for every package, this way anyone can
> start working and we can easily coordinate such great effort.
>
> We are currently trying to switch to github issue tracker, this will a
> good opportunity to start using this new tool.
>
> Thank you John, feel free to pick up the issues, all help is really
> welcome !
>
> Enrico
>
> Il mar 4 lug 2017, 21:47 John Lonergan <jo...@gmail.com> ha
> scritto:
>
>> 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" <eo...@gmail.com> wrote:
>>
>>>
>>>
>>> Il mar 4 lug 2017, 18:06 Dávid Szigecsán <si...@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 <gu...@gmail.com>:
>>>>
>>>> > Those modules are fine, they are rarely touched any way.
>>>> >
>>>> > On Jul 4, 2017 8:57 AM, "Enrico Olivelli" <eo...@gmail.com>
>>>> wrote:
>>>> >
>>>> > > 2017-07-04 16:50 GMT+02:00 Sijie Guo <gu...@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" <eo...@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
>>>
>> --
>
>
> -- Enrico Olivelli
>

Re: BookKeeper and CheckStyle

Posted by Sijie Guo <gu...@gmail.com>.
Awesome! Glad to see the community coordinate the efforts working on this.

Sijie

On Jul 5, 2017 5:48 AM, "Enrico Olivelli" <eo...@gmail.com> wrote:

> David,,
> I see you have created an issue for the first part of bookkeeper-server.
> Maybe you can also create issues for every package, this way anyone can
> start working and we can easily coordinate such great effort.
>
> We are currently trying to switch to github issue tracker, this will a
> good opportunity to start using this new tool.
>
> Thank you John, feel free to pick up the issues, all help is really
> welcome !
>
> Enrico
>
> Il mar 4 lug 2017, 21:47 John Lonergan <jo...@gmail.com> ha
> scritto:
>
>> 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" <eo...@gmail.com> wrote:
>>
>>>
>>>
>>> Il mar 4 lug 2017, 18:06 Dávid Szigecsán <si...@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 <gu...@gmail.com>:
>>>>
>>>> > Those modules are fine, they are rarely touched any way.
>>>> >
>>>> > On Jul 4, 2017 8:57 AM, "Enrico Olivelli" <eo...@gmail.com>
>>>> wrote:
>>>> >
>>>> > > 2017-07-04 16:50 GMT+02:00 Sijie Guo <gu...@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" <eo...@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
>>>
>> --
>
>
> -- Enrico Olivelli
>

Re: BookKeeper and CheckStyle

Posted by Enrico Olivelli <eo...@gmail.com>.
David,,
I see you have created an issue for the first part of bookkeeper-server.
Maybe you can also create issues for every package, this way anyone can
start working and we can easily coordinate such great effort.

We are currently trying to switch to github issue tracker, this will a good
opportunity to start using this new tool.

Thank you John, feel free to pick up the issues, all help is really welcome
!

Enrico

Il mar 4 lug 2017, 21:47 John Lonergan <jo...@gmail.com> ha scritto:

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


-- Enrico Olivelli

Re: BookKeeper and CheckStyle

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

Re: BookKeeper and CheckStyle

Posted by John Lonergan <jo...@gmail.com>.
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" <eo...@gmail.com> wrote:

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