cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Koushik Das <koushik....@citrix.com>
Subject RE: [DISCUSS] PR list length
Date Fri, 17 Jul 2015 11:50:39 GMT
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
Mime
View raw message