Open Stack

On Thu, Aug 21, 2014 at 04:52:37PM -0500, Dolph Mathews wrote:
> On Thu, Aug 21, 2014 at 11:53 AM, Daniel P. Berrange <berrange at redhat.com>
> wrote:
> 
> > On Thu, Aug 21, 2014 at 11:34:48AM -0500, Dolph Mathews wrote:
> > > On Thu, Aug 21, 2014 at 11:21 AM, Daniel P. Berrange <
> > 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."
> >
> 
> > I find the suggestion that reviewers are either "too lazy" or "unwilling"
> > to fix it themselves rather distasteful to be honest.
> >
> 
> I should have followed the above with a gently sprinkling of </sarcasm> and
> </dogfooding>.

Ah, ok that explains it! The perils of communication via email :-)

[snip]

> > I'd only recommend fixing & resubmitting someone else's patch if it is
> > a really trivial thing that needed tweaking before approval for merge,
> > or if they are known to be away for a prolonged time and the patch was
> > blocking other important pending work.
> >
> 
> This is a great general rule. But with enough code reviews, there will be
> exceptions!

Of course. I'd always call these guidelines rather than rules since we
always want to retain the flexibility to ignore guidelines in exceptional
cases where they are counterproductive :-)

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