launchpad-dev team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

On Wed, Oct 20, 2010 at 9:51 PM, Martin Pool <mbp@xxxxxxxxxxxxx> wrote:
> On 21 October 2010 02:18, Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx> wrote:
>> To be clear about my skepticism, it's that I think most of our
>> friction over reviews is that we don't have a shared understanding of
>> the purpose of a review or even what code quality means (e.g. some
>> think lint and formatting, others architectural issues, others module
>> or function design, etc.), so I think all this experiment will show us
>> is the number of people who opt out of review. :-)  Also, I don't have
>> some great fear of plummeting code quality.  We're all responsible
>> adults here. :-)  I guess it's more that I worry about inconsistent
>> practices and statements, i.e. "we value peer-reviewed code, but you
>> don't have to do it if you don't want to."
>
> 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.
>
> I think people can have some fear that if reviews aren't strict, the
> code will become incoherent or hard to read.   However you have to
> remember that new contributors who aren't familiar with our practices
> are very likely to be only writing small bits of code.  The bulk of
> the code will be done by core contributors and so the question is to
> what extent they agree on and care about certain qualities, and how
> much they feel they have the time/environment that lets them make
> things nice.
>
> Code reviews can be a chance to show people how much you care about
> helping them learn the code and get their change in.  I feel this is
> the best way to get them to come back and improve more things - much
> better than holding the original patch hostage until more work is
> done.
>
> At least as a thought experiment they could be optional for everyone.
> If you have to force people to do them it suggests to me that they are
> either taking too long or not focussing on things the contributor
> actually finds useful.
>
> iow: pace jml, "I love your patch, and you too."
>

Thanks for writing this, Martin.  I like the sentiment that reviews
are more about contributors than contributions, and I agree completely
with what you've written here.  I think this is why people can find
our reviews frustrating, I imagine, because they can come off as
blocking mechanisms rather than enabling mechanisms.

Cheers,
deryck

-- 
Deryck Hodge
https://launchpad.net/~deryck
http://www.devurandom.org/