Open Stack

On 11/07/2013 01:56 PM, James Bottomley wrote:
> 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.

+1 to this.

Realistically if I have questions in the code and am unsure how I'd
score it I'll often leave a 0 review with questions inline to help me
understand. I consider this generally good form, and it's helpful to
everyone in the learning process about the code.

    -Sean

-- 
Sean Dague
http://dague.net

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20131108/3e6d655d/attachment.pgp>