On 06/17/2014 07:23 AM, Daniel P. Berrange wrote: > On Tue, Jun 17, 2014 at 11:04:17AM +0100, Matthew Booth wrote: >> We all know that review can be a bottleneck for Nova patches.Not only >> that, but a patch lingering in review, no matter how trivial, will >> eventually accrue rebases which sap gate resources, developer time, and >> will to live. >> >> It occurs to me that there are a significant class of patches which >> simply don't require the attention of a core reviewer. Some examples: >> >> * Indentation cleanup/comment fixes >> * Simple code motion >> * File permission changes >> * Trivial fixes which are obviously correct >> >> The advantage of a core reviewer is that they have experience of the >> whole code base, and have proven their ability to make and judge core >> changes. However, some fixes don't require this level of attention, as >> they are self-contained and obvious to any reasonable programmer. >> >> Without knowing anything of the architecture of gerrit, I propose >> something along the lines of a '+1 (trivial)' review flag. If a review >> gained some small number of these, I suggest 2 would be reasonable, it >> would be equivalent to a +2 from a core reviewer. The ability to set >> this flag would be a privilege. However, the bar to gaining this >> privilege would be low, and preferably automatically set, e.g. 5 >> accepted patches. It would be removed for abuse. >> >> Is this practical? Would it help? > > You are right that some types of fix are so straightforward that > most reasonable programmers can validate them. At the same time > though, this means that they also don't really consume significant > review time from core reviewers. So having non-cores' approve > trivial fixes wouldn't really reduce the burden on core devs. > > The main positive impact would probably be a faster turn around > time on getting the patches approved because it is easy for the > trivial fixes to drown in the noise. > > IME any non-trivial change to gerrit is just not going to happen > in any reasonably useful timeframe though. Perhaps an alternative > strategy would be to focus on identifying which the trivial > fixes are. If there was an good way to get a list of all pending > trivial fixes, then it would make it straightforward for cores > to jump in and approve those simple patches as a priority, to avoid > them languishing too long. > > If would be nice if gerrit had simple keyword tagging so any reviewer > can tag an existing commit as "trivial", but that doesn't seem to > exist as a concept yet. > > So an alternative perhaps submit trivial stuff using a well known > topic eg > > # git review --topic trivial > > Then you can just query all changes in that topic to find easy stuff > to approve.
It could go in the commit message: TrivialFix Then could be queried with - https://review.openstack.org/#/q/message:TrivialFix,n,z If a reviewer felt it wasn't a trivial fix, they could just edit the commit message inline to drop it out. -Sean -- Sean Dague http://dague.net
signature.asc
Description: OpenPGP digital signature
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev