[openstack-dev] Bad review patterns
dstanek at dstanek.com
Thu Nov 7 02:59:04 UTC 2013
On Wed, Nov 6, 2013 at 7:21 PM, Day, Phil <philip.day at hp.com> wrote:
> > Leaving a mark.
> > ===============
> > You review a change and see that it is mostly fine, but you feel that
> since you
> > did so much work reviewing it, you should at least find
> > *something* wrong. So you find some nitpick and -1 the change just so
> > they know you reviewed it.
> > This is quite obvious. Just don't do it. It's OK to spend an hour
> > something, and then leaving no comments on it, because it's simply fine,
> > because we had to means to test someting (see the first pattern).
> Another one that comes into this category is adding a -1 which just says
> "I agree with
> the other -1's in here". If you have some additional perspective and can
> expand on
> it then that's fine - otherwise it adds very little and is just review
> count chasing.
> It's an unfortunate consequence of counting and publishing review stats
> that having
> such a measure will inevitable also drive behavour.
I agree that having no comment with a +1 is OK. If I think the code looks
good I'll +1 it. If I think the code could be better I'll -1 it and leave
I don't think that leaving a -1 with a 'what they said' comment is a bad
thing. As the developer writing the patch it's helpful to know there is a
consensus and it's not just one person requesting a change.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the OpenStack-dev