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.hoogl...@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 <raj...@apache.org> wrote: > I agree with wilder. > > ~Rajani > > On Thu, Jul 16, 2015 at 12:08 PM, Wilder Rodrigues < > wrodrig...@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....@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:w...@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
