[openstack-dev] [Glance] Nitpicking in code reviews
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
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:
>>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  and bikeshedding  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,
>>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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 819 bytes
Desc: not available
More information about the OpenStack-dev