[openstack-dev] Please stop reviewing code while asking questions

Joe Gordon joe.gordon0 at gmail.com
Fri Apr 24 18:01:53 UTC 2015


On Fri, Apr 24, 2015 at 10:31 AM, Doug Hellmann <doug at doughellmann.com>
wrote:

> Excerpts from Amrith Kumar's message of 2015-04-24 15:02:01 +0000:
> > There have been many replies on this thread, I'll just reply to this one
> rather than trying to reply piecemeal.
> >
> > Doug, there's asking a question because something is unclear (implying
> that the code is needlessly complex, missing a comment, is unintuitive,
> ...). I believe that this most definitely warrants a -1 as you describe
> because it is indicative that the code, once submitted, would be hard for a
> future reader to follow.
> >
> > In my mind, the yardstick has been, and continues to be this; would a
> reasonable person believe that the patch set as presented should be allowed
> to merge.
> >
> > - If I can answer that question unambiguously, and unequivocally with a
> NO, then I will score the patch with a negative score.
> >
> > - If I can answer that question unambiguously, and unequivocally with a
> YES, then I will score the patch with a positive score.
> >
> > - For anything else, I'll use a 0.
>
> I've run into too many cases where a "trivial" change has an
> unintended consequence, so I suppose I'm more conservative with my
> code reviews. I don't use 0 very often at all, not because of any
> stats counting, but because I don't assume that conveys any information
> to the author or other reviewers. I vote the way I mean for my
> comments to be taken, so I use -1 to indicate that more work is
> needed, even if that work is just explaining something better or
> demonstrating that an edge case is going to be handled.
>
>
When I get a -1 on one of my patches with a question, I personally treat it
as a short coming of the commit message. To often in the past I have looked
at a file, and in trying to figure out why that line is there I do a git
blame only to see a useless commit message with me as the author.


> >
> > If there was a patch to make reviewstats count +0, I'd support that. But
> I would also like to understand what the differences are between
> reviewstats and stackalytics. Ideally I would like stackalytics to count
> 0's as well. If you can take the time to review a patch, you should get
> credit for it (no matter what tool is used to count).
> >
> > I would support changes to both reviewstats and stackalytics to do the
> following.
>
> I'm not sure where all of the interest in stats counting comes from, but
> it's definitely not a motivation of my own behavior.
>
> Doug
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150424/a513eea2/attachment.html>


More information about the OpenStack-dev mailing list