[openstack-dev] [nova] A modest proposal to reduce reviewer load

Sean Dague sean at dague.net
Wed Jun 18 11:25:16 UTC 2014


On 06/18/2014 04:46 AM, Daniel P. Berrange wrote:
> On Tue, Jun 17, 2014 at 12:55:26PM -0400, Russell Bryant wrote:
>> On 06/17/2014 12:22 PM, Joe Gordon wrote:
>>>
>>>
>>>
>>> On Tue, Jun 17, 2014 at 3:56 AM, Duncan Thomas <duncan.thomas at gmail.com
>>> <mailto:duncan.thomas at gmail.com>> wrote:
>>>
>>>     A far more effective way to reduce the load of trivial review issues
>>>     on core reviewers is for none-core reviewers to get in there first,
>>>     spot the problems and add a -1 - the trivial issues are then hopefully
>>>     fixed up before a core reviewer even looks at the patch.
>>>
>>>     The fundamental problem with review is that there are more people
>>>     submitting than doing regular reviews. If you want the review queue to
>>>     shrink, do five reviews for every one you submit. A -1 from a
>>>     none-core (followed by a +1 when all the issues are fixed) is far,
>>>     far, far more useful in general than a +1 on a new patch.
>>>
>>>
>>> ++
>>>
>>> I think this thread is trying to optimize for the wrong types of
>>> patches.  We shouldn't be focusing on making trivial patches land
>>> faster, but rather more important changes such as bugs and blueprints.
>>> As some simple code motion won't directly fix any users issue such as
>>> bugs or missing features.
>>
>> In fact, landing easier and less important changes causes churn in the
>> code base can make the more important bugs and blueprints even *harder*
>> to get done.
> 
> None the less I think it is worthwhile having a way to tag trivial
> bugs so we can easily identify them. IMHO if there's a way we can
> improve turnaround time on such bugs it is worth it, if only to
> stop authors getting depressed with the wait for trivial/obvious
> fixes.

I'm definitely on that side of the fence. Actual trivial bug fix changes
(where we're talking about a couple of targeted lines) aren't very
dangerous from a merge conflict stand point.

It's the cross cutting stuff like hacking/pep8 clean ups that wreck us
on merge conflicts.

And I do agree that turn around time is important, especially for new
developers. Because you have context on an issue, and if it takes weeks
for people to get to it, by the time they do you are off to something else.

Honestly, what I really wish gerrit would do is sort based on # of lines
changed, from small to large. That would provide a good feedback loop to
make things reviewable chunks.

	-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/20140618/add04bba/attachment.pgp>


More information about the OpenStack-dev mailing list