Open Stack

On Wed, Oct 16, 2013 at 12:59:26PM +0000, Alessandro Pilotti wrote:
> 
> When somebody (especially a core reviewer) puts a -1 and a new patch is committed to address it, 
> I noticed that other reviewers wait for the guy that put the -1 to say something before +1/+2 it. 

I think that depends on the scope of the change the reviewer asked for. It is
normally easy for any other reviewer to identify whether the -1 was properly
addressed and as such there's no need to block on the original reviewer
adding +1. Any core reviewer should be capable of evaluating if a review
point was addressed. Only if the code change was fairly complicated and/or
controversial might it be worth blocking on the original reviewer. I tend to
take such a pragmatic approach when considering whether to wait for the original
reviewer to add a +1 or not.

> My feeling on this is that if somebody reviews a patch (positively or negatively)
> he/she should also keep on with it (in a timely manner) until it is merged or
> clearly stating that there's no interest in reviewing it further. This is especially
> true for core revs as other reviewers tend to be shy and avoid contradicting a core
> rev, generating further delays. 

As above, I don't think we should block on waiting for original reviewers
to review followups, nor require them to, as it is an inefficient way
of working. Any core reviewer should be capable of reviewing any patch
at any stage of its life, unless it is a very controversial change. Forcing
reviewers to keep up with all versions of a patch will never work out in
practice whether we want it or not.

Non-core reviewers should be encouraged to speak up - by doing so it will
improve quality of reviewers and help us identify non-core reviewers who
are good candidates for promotion.


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 :|