You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Daan Hoogland <da...@gmail.com> on 2015/07/14 13:56:41 UTC
[DISCUSS] PR list length
H,
It is a concern to me that the list of PRs on our github page is
beyond a single page (maybe configurable but now a t a very reasonable
25). I think we should adhere to a discipline of not having any PRs
open after the weekend. This is putting a very strong statement
outthere, I realize. A PR might be under heavy construction and very
big (which should result in a discussion about splitting it!) I
Discussed this with Wilder and the idea popped up to have a seven day
limit on (undicussed) PRs. This is however more sensible from an
automation point of view then from a development discipline point of
view. A regular cycle of closing-or-discarding PRs makes more sense.
The list of PRs remaining open is slowly but very steadily growing
over time.
thoughts?
--
Daan
Re: [DISCUSS] PR list length
Posted by Rohit Yadav <ro...@shapeblue.com>.
On 14-Jul-2015, at 5:26 pm, Daan Hoogland <da...@gmail.com>> wrote:
H,
It is a concern to me that the list of PRs on our github page is
beyond a single page (maybe configurable but now a t a very reasonable
25). I think we should adhere to a discipline of not having any PRs
open after the weekend. This is putting a very strong statement
outthere, I realize. A PR might be under heavy construction and very
big (which should result in a discussion about splitting it!) I
Discussed this with Wilder and the idea popped up to have a seven day
limit on (undicussed) PRs. This is however more sensible from an
automation point of view then from a development discipline point of
view. A regular cycle of closing-or-discarding PRs makes more sense.
The list of PRs remaining open is slowly but very steadily growing
over time.
thoughts?
Sounds good, 1 week+ open PR which has green checks (rat, jenkins build, travis), no objections from anybody (and perhaps with at least one LGTM) should be merged on master.
Regards,
Rohit Yadav
Software Architect, ShapeBlue
[cid:9DD97B41-04C5-45F0-92A7-951F3E962F7A]
M. +91 88 262 30892 | rohit.yadav@shapeblue.com<ma...@shapeblue.com>
Blog: bhaisaab.org<http://bhaisaab.org> | Twitter: @_bhaisaab
Find out more about ShapeBlue and our range of CloudStack related services
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge - rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.
Re: [DISCUSS] PR list length
Posted by Daan Hoogland <da...@gmail.com>.
I see your point Koushik, but as a matter of spreading the knowledge I
think it is best to have non-expert give a second ok. In those cases
extra explanation will be needed. That doesn't hurt either. If we all
have the discipline to work on reviews once a week, we are spreading
knowledge and keeping the list short.
If we demand this of ourselves we are also demanding of ourselves to
keep specialised code as simple as possible. We are preventing the
creation of niches.
What Wilder suggests id to drop the PR after a specified time instead
of letting it pass. This forces whoever thinks it is important enough
to share knowledge.
thoughts?
On Fri, Jul 17, 2015 at 1:50 PM, Koushik Das <ko...@citrix.com> wrote:
> For any reviews there are 2 aspects - general coding guidelines and domain knowledge. Based on the current backlog of PRs, I feel that whenever some domain knowledge is required to review a PR and there are not enough reviewers for that, the PR waits for a longer time. That's why I had suggested that if a bug fix PR is green (as per travis) but there isn't enough LGTMs beyond a specified time then there should be some exception to the rule.
>
> -----Original Message-----
> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
> Sent: Friday, 17 July 2015 16:50
> To: dev
> Subject: Re: [DISCUSS] PR list length
>
> I agree as well and since I started this thread a lot of work is being done to keep the list as short as possible. I am still worried that the list is growing, it has over a few weeks and though the growth stopped during the lifetime of this thread I hope we can keep up the discipline to keep it short. At the moment it depends on us few which is not a future proof situation.
>
> On Fri, Jul 17, 2015 at 1:08 PM, Rajani Karuturi <ra...@apache.org> wrote:
>> I agree with wilder.
>>
>> ~Rajani
>>
>> On Thu, Jul 16, 2015 at 12:08 PM, Wilder Rodrigues <
>> WRodrigues@schubergphilis.com> wrote:
>>
>>> We should stick to the 2 LGTM. No matter if that a bug fix or a new
>>> feature.
>>>
>>> Currently we have PRs where 1 LGTM was given, but them the second
>>> reviewer asked questions and raised concerns which were not answered
>>> accordingly. If the 1 LGTM would have been applied, all concernes would have been ignored.
>>>
>>> IMO, a PR siting for 7 or more days without a reply to the reviewer’s
>>> questions/concerns should be closed without merge. In case it’s
>>> really needed, the author will give it some attention and reopen it.
>>>
>>> Cheers,
>>> Wilder
>>>
>>>
>>> > On 14 Jul 2015, at 14:23, Koushik Das <ko...@citrix.com> wrote:
>>> >
>>> > For bug fixes I feel 1 LGTM should be fine provided there is a Jira
>>> ticket with all details and the request is pending for more than a
>>> specified time (may be 7 days). For new features the existing process
>>> should be fine.
>>> >
>>> > -----Original Message-----
>>> > From: Wido den Hollander [mailto:wido@widodh.nl]
>>> > Sent: Tuesday, 14 July 2015 17:42
>>> > To: dev@cloudstack.apache.org
>>> > Subject: Re: [DISCUSS] PR list length
>>> >
>>> > -----BEGIN PGP SIGNED MESSAGE-----
>>> > Hash: SHA1
>>> >
>>> >
>>> >
>>> > On 14-07-15 13:56, Daan Hoogland wrote:
>>> >> H,
>>> >>
>>> >> It is a concern to me that the list of PRs on our github page is
>>> >> beyond a single page (maybe configurable but now a t a very
>>> >> reasonable 25). I think we should adhere to a discipline of not
>>> >> having any PRs open after the weekend. This is putting a very
>>> >> strong statement outthere, I realize. A PR might be under heavy
>>> >> construction and very big (which should result in a discussion
>>> >> about splitting it!) I Discussed this with Wilder and the idea
>>> >> popped up to have a seven day limit on (undicussed) PRs. This is
>>> >> however more sensible from an automation point of view then from a
>>> >> development discipline point of view. A regular cycle of closing-or-discarding PRs makes more sense.
>>> >> The list of PRs remaining open is slowly but very steadily growing
>>> >> over time.
>>> >>
>>> >> thoughts?
>>> >>
>>> >
>>> > I agree. I took the time to look at most of the PRs this morning,
>>> > but a
>>> lot of stuff is about code I don't know, so it's hard to vote LGTM on
>>> such a PR.
>>> >
>>> > But I agree, 25+ PRs open is not good.
>>> >
>>> > Wido
>>> > -----BEGIN PGP SIGNATURE-----
>>> > Version: GnuPG v1
>>> >
>>> > iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
>>> > 8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
>>> > z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
>>> > CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
>>> > ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
>>> > nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
>>> > 0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
>>> > 2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
>>> > JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
>>> > BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
>>> > OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
>>> > +Lg/AwfNPbXnNAFVQlrF
>>> > =rzba
>>> > -----END PGP SIGNATURE-----
>>>
>>>
>
>
>
> --
> Daan
--
Daan
RE: [DISCUSS] PR list length
Posted by Koushik Das <ko...@citrix.com>.
For any reviews there are 2 aspects - general coding guidelines and domain knowledge. Based on the current backlog of PRs, I feel that whenever some domain knowledge is required to review a PR and there are not enough reviewers for that, the PR waits for a longer time. That's why I had suggested that if a bug fix PR is green (as per travis) but there isn't enough LGTMs beyond a specified time then there should be some exception to the rule.
-----Original Message-----
From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
Sent: Friday, 17 July 2015 16:50
To: dev
Subject: Re: [DISCUSS] PR list length
I agree as well and since I started this thread a lot of work is being done to keep the list as short as possible. I am still worried that the list is growing, it has over a few weeks and though the growth stopped during the lifetime of this thread I hope we can keep up the discipline to keep it short. At the moment it depends on us few which is not a future proof situation.
On Fri, Jul 17, 2015 at 1:08 PM, Rajani Karuturi <ra...@apache.org> wrote:
> I agree with wilder.
>
> ~Rajani
>
> On Thu, Jul 16, 2015 at 12:08 PM, Wilder Rodrigues <
> WRodrigues@schubergphilis.com> wrote:
>
>> We should stick to the 2 LGTM. No matter if that a bug fix or a new
>> feature.
>>
>> Currently we have PRs where 1 LGTM was given, but them the second
>> reviewer asked questions and raised concerns which were not answered
>> accordingly. If the 1 LGTM would have been applied, all concernes would have been ignored.
>>
>> IMO, a PR siting for 7 or more days without a reply to the reviewer’s
>> questions/concerns should be closed without merge. In case it’s
>> really needed, the author will give it some attention and reopen it.
>>
>> Cheers,
>> Wilder
>>
>>
>> > On 14 Jul 2015, at 14:23, Koushik Das <ko...@citrix.com> wrote:
>> >
>> > For bug fixes I feel 1 LGTM should be fine provided there is a Jira
>> ticket with all details and the request is pending for more than a
>> specified time (may be 7 days). For new features the existing process
>> should be fine.
>> >
>> > -----Original Message-----
>> > From: Wido den Hollander [mailto:wido@widodh.nl]
>> > Sent: Tuesday, 14 July 2015 17:42
>> > To: dev@cloudstack.apache.org
>> > Subject: Re: [DISCUSS] PR list length
>> >
>> > -----BEGIN PGP SIGNED MESSAGE-----
>> > Hash: SHA1
>> >
>> >
>> >
>> > On 14-07-15 13:56, Daan Hoogland wrote:
>> >> H,
>> >>
>> >> It is a concern to me that the list of PRs on our github page is
>> >> beyond a single page (maybe configurable but now a t a very
>> >> reasonable 25). I think we should adhere to a discipline of not
>> >> having any PRs open after the weekend. This is putting a very
>> >> strong statement outthere, I realize. A PR might be under heavy
>> >> construction and very big (which should result in a discussion
>> >> about splitting it!) I Discussed this with Wilder and the idea
>> >> popped up to have a seven day limit on (undicussed) PRs. This is
>> >> however more sensible from an automation point of view then from a
>> >> development discipline point of view. A regular cycle of closing-or-discarding PRs makes more sense.
>> >> The list of PRs remaining open is slowly but very steadily growing
>> >> over time.
>> >>
>> >> thoughts?
>> >>
>> >
>> > I agree. I took the time to look at most of the PRs this morning,
>> > but a
>> lot of stuff is about code I don't know, so it's hard to vote LGTM on
>> such a PR.
>> >
>> > But I agree, 25+ PRs open is not good.
>> >
>> > Wido
>> > -----BEGIN PGP SIGNATURE-----
>> > Version: GnuPG v1
>> >
>> > iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
>> > 8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
>> > z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
>> > CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
>> > ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
>> > nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
>> > 0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
>> > 2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
>> > JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
>> > BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
>> > OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
>> > +Lg/AwfNPbXnNAFVQlrF
>> > =rzba
>> > -----END PGP SIGNATURE-----
>>
>>
--
Daan
Re: [DISCUSS] PR list length
Posted by Daan Hoogland <da...@gmail.com>.
I agree as well and since I started this thread a lot of work is being
done to keep the list as short as possible. I am still worried that
the list is growing, it has over a few weeks and though the growth
stopped during the lifetime of this thread I hope we can keep up the
discipline to keep it short. At the moment it depends on us few which
is not a future proof situation.
On Fri, Jul 17, 2015 at 1:08 PM, Rajani Karuturi <ra...@apache.org> wrote:
> I agree with wilder.
>
> ~Rajani
>
> On Thu, Jul 16, 2015 at 12:08 PM, Wilder Rodrigues <
> WRodrigues@schubergphilis.com> wrote:
>
>> We should stick to the 2 LGTM. No matter if that a bug fix or a new
>> feature.
>>
>> Currently we have PRs where 1 LGTM was given, but them the second reviewer
>> asked questions and raised concerns which were not answered accordingly. If
>> the 1 LGTM would have been applied, all concernes would have been ignored.
>>
>> IMO, a PR siting for 7 or more days without a reply to the reviewer’s
>> questions/concerns should be closed without merge. In case it’s really
>> needed, the author will give it some attention and reopen it.
>>
>> Cheers,
>> Wilder
>>
>>
>> > On 14 Jul 2015, at 14:23, Koushik Das <ko...@citrix.com> wrote:
>> >
>> > For bug fixes I feel 1 LGTM should be fine provided there is a Jira
>> ticket with all details and the request is pending for more than a
>> specified time (may be 7 days). For new features the existing process
>> should be fine.
>> >
>> > -----Original Message-----
>> > From: Wido den Hollander [mailto:wido@widodh.nl]
>> > Sent: Tuesday, 14 July 2015 17:42
>> > To: dev@cloudstack.apache.org
>> > Subject: Re: [DISCUSS] PR list length
>> >
>> > -----BEGIN PGP SIGNED MESSAGE-----
>> > Hash: SHA1
>> >
>> >
>> >
>> > On 14-07-15 13:56, Daan Hoogland wrote:
>> >> H,
>> >>
>> >> It is a concern to me that the list of PRs on our github page is
>> >> beyond a single page (maybe configurable but now a t a very reasonable
>> >> 25). I think we should adhere to a discipline of not having any PRs
>> >> open after the weekend. This is putting a very strong statement
>> >> outthere, I realize. A PR might be under heavy construction and very
>> >> big (which should result in a discussion about splitting it!) I
>> >> Discussed this with Wilder and the idea popped up to have a seven day
>> >> limit on (undicussed) PRs. This is however more sensible from an
>> >> automation point of view then from a development discipline point of
>> >> view. A regular cycle of closing-or-discarding PRs makes more sense.
>> >> The list of PRs remaining open is slowly but very steadily growing
>> >> over time.
>> >>
>> >> thoughts?
>> >>
>> >
>> > I agree. I took the time to look at most of the PRs this morning, but a
>> lot of stuff is about code I don't know, so it's hard to vote LGTM on such
>> a PR.
>> >
>> > But I agree, 25+ PRs open is not good.
>> >
>> > Wido
>> > -----BEGIN PGP SIGNATURE-----
>> > Version: GnuPG v1
>> >
>> > iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
>> > 8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
>> > z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
>> > CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
>> > ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
>> > nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
>> > 0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
>> > 2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
>> > JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
>> > BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
>> > OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
>> > +Lg/AwfNPbXnNAFVQlrF
>> > =rzba
>> > -----END PGP SIGNATURE-----
>>
>>
--
Daan
Re: [DISCUSS] PR list length
Posted by Rajani Karuturi <ra...@apache.org>.
I agree with wilder.
~Rajani
On Thu, Jul 16, 2015 at 12:08 PM, Wilder Rodrigues <
WRodrigues@schubergphilis.com> wrote:
> We should stick to the 2 LGTM. No matter if that a bug fix or a new
> feature.
>
> Currently we have PRs where 1 LGTM was given, but them the second reviewer
> asked questions and raised concerns which were not answered accordingly. If
> the 1 LGTM would have been applied, all concernes would have been ignored.
>
> IMO, a PR siting for 7 or more days without a reply to the reviewer’s
> questions/concerns should be closed without merge. In case it’s really
> needed, the author will give it some attention and reopen it.
>
> Cheers,
> Wilder
>
>
> > On 14 Jul 2015, at 14:23, Koushik Das <ko...@citrix.com> wrote:
> >
> > For bug fixes I feel 1 LGTM should be fine provided there is a Jira
> ticket with all details and the request is pending for more than a
> specified time (may be 7 days). For new features the existing process
> should be fine.
> >
> > -----Original Message-----
> > From: Wido den Hollander [mailto:wido@widodh.nl]
> > Sent: Tuesday, 14 July 2015 17:42
> > To: dev@cloudstack.apache.org
> > Subject: Re: [DISCUSS] PR list length
> >
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> >
> >
> > On 14-07-15 13:56, Daan Hoogland wrote:
> >> H,
> >>
> >> It is a concern to me that the list of PRs on our github page is
> >> beyond a single page (maybe configurable but now a t a very reasonable
> >> 25). I think we should adhere to a discipline of not having any PRs
> >> open after the weekend. This is putting a very strong statement
> >> outthere, I realize. A PR might be under heavy construction and very
> >> big (which should result in a discussion about splitting it!) I
> >> Discussed this with Wilder and the idea popped up to have a seven day
> >> limit on (undicussed) PRs. This is however more sensible from an
> >> automation point of view then from a development discipline point of
> >> view. A regular cycle of closing-or-discarding PRs makes more sense.
> >> The list of PRs remaining open is slowly but very steadily growing
> >> over time.
> >>
> >> thoughts?
> >>
> >
> > I agree. I took the time to look at most of the PRs this morning, but a
> lot of stuff is about code I don't know, so it's hard to vote LGTM on such
> a PR.
> >
> > But I agree, 25+ PRs open is not good.
> >
> > Wido
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1
> >
> > iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
> > 8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
> > z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
> > CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
> > ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
> > nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
> > 0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
> > 2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
> > JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
> > BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
> > OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
> > +Lg/AwfNPbXnNAFVQlrF
> > =rzba
> > -----END PGP SIGNATURE-----
>
>
Re: [DISCUSS] PR list length
Posted by Wilder Rodrigues <WR...@schubergphilis.com>.
We should stick to the 2 LGTM. No matter if that a bug fix or a new feature.
Currently we have PRs where 1 LGTM was given, but them the second reviewer asked questions and raised concerns which were not answered accordingly. If the 1 LGTM would have been applied, all concernes would have been ignored.
IMO, a PR siting for 7 or more days without a reply to the reviewer’s questions/concerns should be closed without merge. In case it’s really needed, the author will give it some attention and reopen it.
Cheers,
Wilder
> On 14 Jul 2015, at 14:23, Koushik Das <ko...@citrix.com> wrote:
>
> For bug fixes I feel 1 LGTM should be fine provided there is a Jira ticket with all details and the request is pending for more than a specified time (may be 7 days). For new features the existing process should be fine.
>
> -----Original Message-----
> From: Wido den Hollander [mailto:wido@widodh.nl]
> Sent: Tuesday, 14 July 2015 17:42
> To: dev@cloudstack.apache.org
> Subject: Re: [DISCUSS] PR list length
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
>
> On 14-07-15 13:56, Daan Hoogland wrote:
>> H,
>>
>> It is a concern to me that the list of PRs on our github page is
>> beyond a single page (maybe configurable but now a t a very reasonable
>> 25). I think we should adhere to a discipline of not having any PRs
>> open after the weekend. This is putting a very strong statement
>> outthere, I realize. A PR might be under heavy construction and very
>> big (which should result in a discussion about splitting it!) I
>> Discussed this with Wilder and the idea popped up to have a seven day
>> limit on (undicussed) PRs. This is however more sensible from an
>> automation point of view then from a development discipline point of
>> view. A regular cycle of closing-or-discarding PRs makes more sense.
>> The list of PRs remaining open is slowly but very steadily growing
>> over time.
>>
>> thoughts?
>>
>
> I agree. I took the time to look at most of the PRs this morning, but a lot of stuff is about code I don't know, so it's hard to vote LGTM on such a PR.
>
> But I agree, 25+ PRs open is not good.
>
> Wido
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
>
> iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
> 8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
> z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
> CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
> ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
> nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
> 0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
> 2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
> JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
> BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
> OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
> +Lg/AwfNPbXnNAFVQlrF
> =rzba
> -----END PGP SIGNATURE-----
RE: [DISCUSS] PR list length
Posted by Koushik Das <ko...@citrix.com>.
For bug fixes I feel 1 LGTM should be fine provided there is a Jira ticket with all details and the request is pending for more than a specified time (may be 7 days). For new features the existing process should be fine.
-----Original Message-----
From: Wido den Hollander [mailto:wido@widodh.nl]
Sent: Tuesday, 14 July 2015 17:42
To: dev@cloudstack.apache.org
Subject: Re: [DISCUSS] PR list length
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 14-07-15 13:56, Daan Hoogland wrote:
> H,
>
> It is a concern to me that the list of PRs on our github page is
> beyond a single page (maybe configurable but now a t a very reasonable
> 25). I think we should adhere to a discipline of not having any PRs
> open after the weekend. This is putting a very strong statement
> outthere, I realize. A PR might be under heavy construction and very
> big (which should result in a discussion about splitting it!) I
> Discussed this with Wilder and the idea popped up to have a seven day
> limit on (undicussed) PRs. This is however more sensible from an
> automation point of view then from a development discipline point of
> view. A regular cycle of closing-or-discarding PRs makes more sense.
> The list of PRs remaining open is slowly but very steadily growing
> over time.
>
> thoughts?
>
I agree. I took the time to look at most of the PRs this morning, but a lot of stuff is about code I don't know, so it's hard to vote LGTM on such a PR.
But I agree, 25+ PRs open is not good.
Wido
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
+Lg/AwfNPbXnNAFVQlrF
=rzba
-----END PGP SIGNATURE-----
Re: [DISCUSS] PR list length
Posted by Wido den Hollander <wi...@widodh.nl>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 14-07-15 13:56, Daan Hoogland wrote:
> H,
>
> It is a concern to me that the list of PRs on our github page is
> beyond a single page (maybe configurable but now a t a very
> reasonable 25). I think we should adhere to a discipline of not
> having any PRs open after the weekend. This is putting a very
> strong statement outthere, I realize. A PR might be under heavy
> construction and very big (which should result in a discussion
> about splitting it!) I Discussed this with Wilder and the idea
> popped up to have a seven day limit on (undicussed) PRs. This is
> however more sensible from an automation point of view then from a
> development discipline point of view. A regular cycle of
> closing-or-discarding PRs makes more sense. The list of PRs
> remaining open is slowly but very steadily growing over time.
>
> thoughts?
>
I agree. I took the time to look at most of the PRs this morning, but
a lot of stuff is about code I don't know, so it's hard to vote LGTM
on such a PR.
But I agree, 25+ PRs open is not good.
Wido
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBAgAGBQJVpPx1AAoJEAGbWC3bPspC5hsQAK1GxhgnHleFvexMOWOxZA3v
8XfR3Sh78DJZvG9hjY9eP4TauWJ6mBoVR5Mxe9M0eWqJ2Uy28PacIaUq4LXfrAY9
z5c+iq4Whi+FUz5mMtmL6x/3MqBlN8Ag9TDnZVE/pwDB1g8m27l23NhK6c5tKpXt
CWh7xTqtCDVGnAO8eA1kk7aLicj3Wd8XaQzR4W7xDxf4XSN6lXEMfnFenD6ShqcA
ktHOwI8r7hFt/M6+eZ7YmBF3dosw0mMH1lgBKaq+jEMSjHJWyVUu4UHxsx1Z9Fup
nyYEEx8U5nYCgl72Zvmtvzth3Es2LoKy1ly19r6YlycMPtqO1T51qcwq3zcdKrpG
0pRPnEuQhMhUhJvuKOd05pEvISOf8Eilm+3k9W9ZxxYtgCe2cqgrn+0/60Uw0fzG
2U2lWlO4p4tYOKUbTSZTqsjYeeA0FvLV1Ib0wq8rwyZTHpxVpdWaz2lR2X3SltZH
JJJsOdtMUxV3lzIBSL7fKjVi9TqbSgrd6QKio3jBl34cw8PStWIRZilIq+fglRnx
BC0epH1YJAB3BzIeChe1kHKzrqADgo0arJt8N4n/Lza6+kW68k7hDx195XUgo3c0
OsKm2jkoo7JtURaOo6/lF+tFBngYdgTyWCFSer/UReycx/xnZcSI0DIz+QbYMOrj
+Lg/AwfNPbXnNAFVQlrF
=rzba
-----END PGP SIGNATURE-----