Open Stack

On Thu, 2013-11-07 at 00:21 +0000, Day, Phil 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.

Perhaps a source of the problem is early voting.  Feeling pressure to
add a +/-1 (or even 2) without fully asking what's going on in the code
leads to premature adjudication.

For instance, I have to do a lot of reviews in SCSI; I'm a subject
matter expert, so I do recognise some bad coding patterns and ask for
them to be changed unconditionally, but a lot of the time I don't
necessarily understand why the code was done in the way it was, so I
ask.  Before I get the answer I'm not really qualified to judge the
merits.

James