PolicyAndProcess/OptionalReviews

Not logged in - Log In / Register

Revision 12 as of 2010-11-23 01:09:19

Clear message

Process Overview

Some reviews are not needed and eligible developers can choose to have branches land without formal review.

Process Description

Launchpad reviewers can choose to land branches without review. Only 'code' reviews can be made optional. Javascript UI and Database patches are out of scope of the experiment.

For the first two weeks (until 12 Nov 2010) all unreviewed landings will be reviewed post landing by Robert.

Each month, all code reviewers will be assigned one unreviewed landed to do a post hoc review as part of assessing the experiment.

Production problems which are tracked to unreviewed landings will also be input into assessing the experiment.

Rationale

Not all changes benefit from code review / are high enough risk to need formal review. For instance (not exhaustive):

For many of these things a review may add value - but less value than doing the review uses up. We want reviews to be a net win for the effort being put into developing Launchpad, and so we should, once we're comfortable people know how things should be, allow them to decide if a particular change is an improvement on its own, or an improvement that also /needs/ other input before landing.

Triggers

Experienced developer (someone currently working on LP who has been doing so for the last 3 months) landing a code change (not UI or Database, for now : evaluate after assessing the success of optional code reviews).

Activities

  1. Submit the branch to create an MP (our toolchains can look at this and it provides a location for a post landing review if the branch has that done to it.)

  2. Self review without a review type.

  3. Land via the normal landing process.

Postreviewed landings (week 4) 11957 to ...

revno

branch

reviewer

would-review-be-worth-it

comments

11959

devel

lifeless

No

Tweak log levels to eliminate some unneeded OOPSes.

Postreviewed landings (week 3) 11927 to 11956

11955

devel

lifeless

Yes

Delete a test which was broken by a bad landing, but actually has (unclear) value.

11954

devel

lifeless

No

Disable a spuriously failing test.

11947

devel

lifeless

No

Reduce oopses by fixing log levels.

11944

devel

lifeless

No

Fix landing where concurrent patches interacted.

11940

devel

lifeless

No

Code in live use copied back into the source tree.

11936

devel

-

No

Apply fix prepped by stub for Person:+commentedbugs timeouts.

11930

devel

lifeless

No

Revert 11917 now its not needed.

Postreviewed landings (week 2) 11876 to 11926

11917

devel

lifeless

No

Fix the fallout from 11905 for staging.

11914

devel

-

No

Fixes prior bad landing that passed review.

11906

devel

lifeless

No

Testfix a bad landing.

11905

devel

lifeless

No

Apocalypse progress - broke staging, but review wouldn't have caught it.

11893

devel

lifeless

No

Fix a missing DB permission showing up in OOPS.

11887

devel

lifeless

No

Bump a dependency version, cleanup code as a result.

11877

devel

-

No

Bump a dependency version.

11876

devel

lifeless

No

Test improvement, was mailed to the list.

Postreviewed landings (week 1) 11823 to 11875

revno

branch

reviewer

would-review-be-worth-it

comments

11874

devel

lifeless

No

Updates a dependency.

11855

devel

lifeless

No

Clear fix with no alternative implementations.

11851

devel

lifeless

Yes

Added a edit-subscription facility for existing subscriptions - review caught a mismatch between subscribe and edit-subscription models.

11842

devel

lifeless

No

Fixed up a XXX landed while a dependent layer was fixed.

11836

devel

lifeless

No

Simple drop of unneeded dep combined with a testfix to trigger buildbot

11827

devel

lifeless

No

Totally mechanical, restructure along already agreed lines

11823

devel

lifeless

No

A testfix - fixing something landed without ec2land