[openstack-dev] Criteria for giving a -1 in a review

Daniel P. Berrange berrange at redhat.com
Thu Aug 21 17:00:15 UTC 2014


On Thu, Aug 21, 2014 at 12:42:43PM -0400, Adam Young wrote:
> On 08/21/2014 12:34 PM, Dolph Mathews wrote:
> >
> >On Thu, Aug 21, 2014 at 11:21 AM, Daniel P. Berrange <berrange at redhat.com
> ><mailto:berrange at redhat.com>> wrote:
> >
> >    On Thu, Aug 21, 2014 at 05:05:04PM +0100, Matthew Booth wrote:
> >    > "I would prefer that you didn't merge this."
> >    >
> >    > i.e. The project is better off without it.
> >
> >    A bit off topic, but I've never liked this message that gets added
> >    as it think it sounds overly negative. It would better written
> >    as
> >
> >      "This patch needs further work before it can be merged"
> >
> >
> >++ "This patch needs further work before it can be merged, and as a
> >reviewer, I am either too lazy or just unwilling to checkout your patch
> >and fix those issues myself."
> 
> Heh...well, there are a couple other aspects:
> 
> 1.  I am unsure if my understanding is correct.  I'd like to have some
> validation, and, if I am wrong, I'll withdraw the objections.
> 
> 2.  If I make the change, I can no longer +2/+A the review.  If you make the
> change, I can approve it.

If it is something totally minor like a typo fix, or docs grammar
fix or whitespace cleanup it is reasonable to +2/+A something that
you took over from the original author, but that would be a pretty
rare scenario in general. Certainly a change which had any kind of
a functional impact, I'd not be happy with a person +2/+A'ing their
re-post.

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



More information about the OpenStack-dev mailing list