cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [DISCUSS] PR list length
Date Fri, 17 Jul 2015 12:49:44 GMT
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 <koushik.das@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 <rajani@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 <koushik.das@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

Mime
View raw message