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 [email protected] 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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
