[openstack-dev] [Glance] Nitpicking in code reviews

Flavio Percoco flavio at redhat.com
Thu Mar 12 14:07:30 UTC 2015

On 11/03/15 15:06 -1000, John Bresnahan wrote:
>FWIW I agree with #3 and #4 but not #1 and #2.  Spelling is an easy 
>enough thing to get right and speaks to the quality standard to which 
>the product is held even in commit messages and comments (consider the 
>'broken window theory').  Of course everyone makes mistakes (I am a 
>terrible speller) but correcting a spelling error should be a trivial 
>matter.  If a reviewer notices a spelling error I would expect them to 
>point it.

I'd agree depending on the status of the patch. If the patch has
already 2 +2s and someone blocks it because of a spelling error, then
the cost of fixing it, running the CI jobs and getting the reviews
again is higher than living with a simple typo.

Process and rules are good but we must evaluate them in a case by case
basis to make sure we're not blocking important work on things that
are not that relevant after all.

>On 3/11/15 2:22 PM, Kuvaja, Erno wrote:
>>Hi all,
>>Following the code reviews lately I’ve noticed that we (the fan club
>>seems to be growing on weekly basis) have been growing culture of
>>nitpicking [1] and bikeshedding [2][3] over almost every single change.
>>Seriously my dear friends, following things are not worth of “-1” vote
>>if even a comment:
>>1)Minor spelling errors on commit messages (as long as the message comes
>>through and flags are not misspelled).
>>2)Minor spelling errors on comments (docstrings and documentation is
>>there and there, but comments, come-on).
>>3)Used syntax that is functional, readable and does not break
>>consistency but does not please your poem bowel.
>>4)Other things you “just did not realize to check if they were there”.
>>After you have gone through the whole change go and look your comments
>>again and think twice if your concern/question/whatsoever was addressed
>>somewhere else than where your first intuition would have dropped it.
>>We have relatively high volume for glance at the moment and this
>>nitpicking and bikeshedding does not help anyone. At best it just
>>tightens nerves and breaks our group. Obviously if there is “you had ONE
>>job” kind of situations or there is relatively high amount of errors
>>combined with something serious it’s reasonable to ask fix the typos on
>>the way as well. The reason being need to increase your statistics,
>>personal perfectionist nature or actually I do not care what; just stop
>>or go and do it somewhere else.

Thanks for bringing all this up, Erno. I've been seeing the same
pattern for all the points you've mentioned above. It's a good
reminder for people to treat each patch individually so we avoid
making our process and rules a pain for everyone.


>>Love and pink ponies,
>>[1] www.urbandictionary.com/define.php?term=nitpicking
>>[2] http://bikeshed.com
>>[3] http://en.wiktionary.org/wiki/bikeshedding
>>OpenStack Development Mailing List (not for usage questions)
>>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>OpenStack Development Mailing List (not for usage questions)
>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe

Flavio Percoco
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150312/075bdf02/attachment.pgp>

More information about the OpenStack-dev mailing list