launchpad-dev team mailing list archive

[Jeroen Vermeulen <jtv@xxxxxxxxxxxxx>]

On 2010-10-20 03:00, Francis J. Lacoste wrote:

> Here Robert is actually proposing an experimental framework to assess our
> beliefs that allowing landing unreviewed changes is actually a bad idea.

Thanks for explaining that.  While I stand by what I said (and will go 
into details below) I don't mean to stifle innovation in our process.


> Also by calling it unreviewed, the claim isn't that the changes are trivial,
> just that the cost of reviewing the changes outweights any benefit.

I don't think we're quite that good at estimating what benefits a review 
can have.  Moreover, we're talking about a license to cut ourselves off 
from the very feedback loop that we learn this skill from.  It takes 
more than 3 months on the project before we can afford to do that. 
Technically I'd say you have to be a full reviewer, but that's still not 
accounting for the inherent self-bias.


> Also, this policy takes a 'we are responsible adults stance'. It trusts that:
>
> - Launchapd engineers know the coding convention, value them and honors them
> (they may make mistake in them sometime, we don't care enough to enfore them
> before landing, let's clean up anything missed post-hoc.)

We're now getting great contributions from bzr people who are expected 
to straddle two enormously different coding standards.  Unless you're 
willing to say outright that we're willing to abandon our coding 
standards (which "we'll just clean it up later" more or less amounts to) 
it's a very bad time to let them slide.

The clash in coding standards makes some reviews very tedious.  It's a 
lot more than "may make mistake in them sometime."  But that's something 
all new contributors to Launchpad have to go through; they're generally 
experienced in other projects but need to adjust to our specific rules. 
 We can't solve it by saying "if it's too hard to get through review, 
just land it without."

Because that's what "the costs of review outweigh any benefits" means 
when you only have your own estimate for the benefits.  An introduction 
period of just 3 months is so short that this experiment seems aimed 
more at sparing new contributors the pain of learning through reviews 
than at allowing experienced contributors to get things done more 
quickly.  Avoiding the pain of learning is a very unhealthy thing to do.


> - We are a mature team that instead of cramming to meet deadline with low
> quality, values flow and fast cadence without compromising code quality. In
> that regards, we remove the most friction in getting changes in and trust that
> engineers will seek out reviews appropriately.

It would be great to reduce friction.  But then I'd want to see a 
last-resort restriction on the optional reviews: bypassing review should 
be possible only if no reviewer is, or is expected to be, available for 
some substantial time.  Otherwise how does the experiment not 
rationalize avoiding criticism as avoiding friction?

Okay, I only talked about friction from handoffs there.  There's 
friction during reviews as well, and not all of that is helpful.  That's 
why I would prefer a "practice as you preach" rule for new coding 
guidelines.  I would like us to accept no new review requirements or 
coding guidelines without a commitment to fix the bulk of existing practice.

So if someone says "comments should be written in dactylic hexameter," 
and the reviewers' meetings agree it's desirable, the next question 
would be "who's willing to fix up the existing comments?"  If we get 
enough volunteers, they update (most of) the codebase and reviewers 
start enforcing the rule.  If not, we abandon the idea.  The goal is to 
end up with a body of review guidelines that you'll pick up through 
osmosis from available examples and surrounding code, not from fighting 
your reviewer.


Jeroen