launchpad-dev team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

On 21 October 2010 11:20, Julian Edwards <julian.edwards@xxxxxxxxxxxxx> wrote:
> On Thursday 21 October 2010 03:51:56 Martin Pool wrote:
>> I said to somebody the other day that I care more about using reviews
>> to improve contributors than to improve code.  What I mean is that
>> it's less important to me that every contribution be as clean as
>> possible than that every contributor internalizes a shared model of
>> what is desirable in code (all the way from whitespace through to
>> architecture), and that they find participation to be fun.
>
> Hell yes.
>
> The two most important things to me about code reviews are, in no particular
> order:
>
>  1. The contributor can learn something from the reviewer
>  2. The reviewer can learn something from the contributor
>
> What I find extremely irritating is nitpicking over minor formatting and
> grammatical issues.  The first thing that pops into my head when someone does
> this, with no other comments about my code, is "you've not really looked at
> what this patch is doing, have you?"

I hope that the second thing you think is "that's unfair of me, bad Julian."

Remember that how you look at the code style really depends upon how
you treat reviews. I was told when I started out that they were about
maintaining code consistency as much as making sure that the patch
worked. That has changed over time but - and this is the real
annoyance here, I suspect - it's not consistent from reviewer to
reviewer. Also, how many times have you read a document in English and
been distracted by errors in spelling, punctuation and grammar? For
some reviewers (for me, at least), errors in the coding style are just
as distracting. Of course, because we do our job right (most of the
time), we make note of those errors and then do a second review where
we ignore them (because we've already dealt with them).

As I said above, though, I'd be interested to see the results of this
experiment (at the moment I'm -0 about it, really).