On Wed, Nov 6, 2013 at 7:21 PM, Day, Phil <philip....@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 > that > > they know you reviewed it. > > > > This is quite obvious. Just don't do it. It's OK to spend an hour > reviewing > > something, and then leaving no comments on it, because it's simply fine, > or > > 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 comments. 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. -- David blog: http://www.traceback.org twitter: http://twitter.com/dstanek www: http://dstanek.com
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev