Open Stack

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.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|