[Python-Dev] Code reviews

[Georg Brandl]

On 01/02/2012 03:41 PM, Antoine Pitrou wrote:
> On Mon, 2 Jan 2012 14:44:49 +1000
> Nick Coghlan <ncoghlan at gmail.com> wrote:
>> 
>> He keeps leaving them out, I occasionally tell him they should always
>> be included (most recently this came up when we gave conflicting
>> advice to a patch contributor).
> 
> Oh, by the way, this is also why I avoid arguing too much about style
> in code reviews. There are two bad things which can happen:
> 
> - your advice conflicts with advice given by another reviewer (perhaps
>   on another issue)
> - the contributor feels drowned under tiresome requests for style
>   fixes ("please indent continuation lines this way")
> 
> Both are potentially demotivating. A contributor can have his/her own
> style if it doesn't adversely affect code quality.

Exactly. Especially for reviews of patches from non-core people, we
should exercise a lot of restraint: as the committers, I think we can be
expected to bite the sour bullet and apply our uniform style (such as
it is).

It is tiresome, if not downright disappointing, to get reviews that
are basically "nothing wrong, but please submit again with one more
empty line between the classes", and definitely not the way to
attract more contributors.

Georg