Diff for "UsingMergeProposals"

Not logged in - Log In / Register

Differences between revisions 4 and 5
Revision 4 as of 2009-08-03 15:53:46
Size: 5565
Editor: cjwatson
Comment: test.py -> bin/test
Revision 5 as of 2012-08-01 20:38:53
Size: 5569
Editor: roel11
Comment: small wording change
Deletions are marked like this. Additions are marked like this.
Line 3: Line 3:
Our goal is two-fold. First, to kill off the PendingReviews wiki page because it sucks to have to edit this page to manage reviews. Second, and more importantly, it is to use more of Launchpad to manage Launchpad's own development. To this end, we are experimenting with dogfooding branch reviews in Launchpad's merge proposals. Our goal is two-fold. First, to kill off the PendingReviews wiki page because it is cumbersome to have to edit this page to manage reviews. Second, and more importantly, it is to use more of Launchpad to manage Launchpad's own development. To this end, we are experimenting with dogfooding branch reviews in Launchpad's merge proposals.
Line 41: Line 41:
  * ''bin/test -vvt'' command     * ''bin/test -vvt'' command
Line 57: Line 57:
      * needs-reply = ''Needs fixing''         * needs-reply = ''Needs fixing''

Using Merge Proposals

Our goal is two-fold. First, to kill off the PendingReviews wiki page because it is cumbersome to have to edit this page to manage reviews. Second, and more importantly, it is to use more of Launchpad to manage Launchpad's own development. To this end, we are experimenting with dogfooding branch reviews in Launchpad's merge proposals.

We are encouraging every developer to submit branches for review using merge proposals. In addition to all the current branch rules (e.g. 800 line maximum), for merge proposals, we are also limiting them to independent branches, or IOW, a branch which is not dependent on another branch. You can use a merge proposal for an on-call review or a normal review. As always, it's still your responsibility to get your branch reviewed.

Here then is a mini-howto on using merge proposals for your branch reviews. You can also read up on the email interface to Merge Proposals at https://help.launchpad.net/Code/Review.

Developers

You are a developer with a branch ready for review. Instead of using PendingReviews, you'd like to submit a merge proposal for your branch.

  1. Link your branch to the bug. You need to do this as part of a commit
    • bzr commit --fixes=lp:123456

    • bzr commit --fixes=lp:123456 --unchanged (if necessary)

  2. Push your branch to Launchpad. DogfoodingStackedLaunchpad

  3. Go to https://code.launchpad.net/launchpad

  4. Click on the branch you want.
  5. Click on the Propose for merging into another branch link.

    • Target branch: Use ~launchpad-pqm/launchpad/devel as the target branch. This should be the default. (If not, be sure you are submitting the branch against Launchpad and not, e.g. Soyuz.)

    • Comment: Enter your cover letter using this template

    • Reviewer: By default, this field will be the ~launchpad team, which

      • is what we want so that they get emails. (Below, we will request a review from a specific person.
    • Click Propose Merge

  6. If you have arranged for an on-call review, click Request another review.

    • Reviewer: Enter the name of the specific reviewer. They

      • will receive an email with a link to the merge proposal.
    • Review type: Enter "code", "db schema", or "UI".

    • Click the Request Review button.

  7. Click the Add a comment link.

    • Paste in the diff for your branch. (Eventually this will be handled automatically.)
    • Click Save Comment.

Be sure to link your branch to the appropriate bug or blueprint! (see above) Also, make sure that bug or blueprint is targeted to the appropriate Launchpad milestone.

Because you are not using the review-submit bzr plugin, make sure your cover letter has all the necessary information, such as:

  • Demo url/instructions
  • bin/test -vvt command

  • Pre-impl information
  • 'make lint' output

Reviewers

  1. Start by going to https://code.launchpad.net/launchpad

  2. At the top of the page, you'll see a count of the number of active reviews. Click on the words active reviews

  3. Look for a Branch Merge Proposal that has a Date Review Requested, but with no votes or comments. This is a branch waiting to be reviewed.
  4. Click on the text in the Branch Merge Proposal column for that branch
  5. Now, you need to go to your own machine, do the merge and grab the pasted diff, reviewing it as you normally would. Or if you prefer to use the email address, just respond to the merge proposal email, reviewing the included diff as you normally would.
  6. Go back to LP for the merge proposal and click on Review next to your name.

    • Reviewer says: Enter the review status. Here is how our internal statuses map to the Merge Proposal statuses:

      • merge-approved = Approve

      • merge-conditional = Approve

      • needs-reply = Needs fixing

    • Comment: Be sure to include merge-approved, etc. in the text comment, and if you're being mentored, be sure to add a star (e.g. *merge-approved). Paste in the rest of your review.

    • Click the Save Review button.

Other notes

Please be sure to report any and all bugs in the merge proposal feature to LP, and please let us know how things are going. This is the way we eventually want to do branch reviews, so your feedback is invaluable to making this feature really rock.

Second experimental results

After Epic (late October 2008), we decided to re-run the experiment since many of the points below have been nicely addressed. We still don't have diffs yet, or direct bzr support, but the system is much more usable now.

First experimental results

On 15-Sep-2008, I proposed to terminate this experiment. It was not exactly a success, but it was not a failure either. We learned many things and gave great feedback to the code guys. Here's the list of things I think we learned; please add your comments here as well!

  • We really need diffs
  • There are usability problems navigating all the pages and information
  • Whiteboards really aren't very good for cover letters
  • We really need to host the code on LP
  • Need bzr support; e.g. automatic branch creation
  • Still too heavyweight to replace PendingReviews

UsingMergeProposals (last edited 2012-08-01 21:25:38 by roel11)