launchpad-dev team mailing list archive

["Francis J. Lacoste" <francis.lacoste@xxxxxxxxxxxxx>]

Hi Jeroen,

I agree that the 3 months period is probably a bad criteria. Let's change the 
criteria to being a reviewer. Which usually happens after being on the job for 
3 months.


-- 
Francis J. Lacoste
francis.lacoste@xxxxxxxxxxxxx

On October 20, 2010, Jeroen Vermeulen wrote:
> 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

Attachment: signature.asc
Description: This is a digitally signed message part.