[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