Open Stack

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140617/afb7711b/attachment.pgp>