Open Stack

On Thu, Aug 21, 2014 at 01:12:16PM -0400, Zane Bitter wrote:
> On 21/08/14 12:21, Daniel P. Berrange 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"
> >
> >as that gives a positive expectation that the work is still
> >wanted by the project in general
> 
> Well, there are two audiences for that message: the developer and the
> reviewer. I can't help thinking that if instead of trying to be positive it
> said what it really means - "Today, I have chosen to obstruct your work for
> the greater good of the project" - we might have a few less -1s for trivial
> issues.
> 
> Maybe, while we're at it, we could stop publishing taxonomies of reasons to
> -1 a patch as if code reviews were a competition to see who can find the
> most.

The reason for putting together examples of reasons to -1 a patch is not
to encourage people to -1 as much as possible. Rather the aim is to get
more consistency in how reviewers treat different types of problems. If
anything the intent of the mail is to actually help reviewers to allow
more trivial problems to get past review. Currently we give contributors
a inconsistent message with one reviewer saying some trivial thing should
be fixed while others will come along and say it is not worth fixing, or
can be fixed later on with a followup.

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