• Immutable Page
• Info
• Attachments
• More Actions: Raw Text Print View Render as Docbook Delete Cache ------------------------ Check Spelling Like Pages Local Site Map ------------------------ Rename Page Delete Page ------------------------ Subscribe User ------------------------ Remove Spam Revert to this revision Package Pages Sync Pages ------------------------ Load Save SlideShow

Changes planned to the Launchpad code review system

Code reviews in Launchpad are based on branches. This is not going to change at this time.

Diff Changes

Right now proposals have two associated diffs.

• The review diff - doesn't change, generated on submission, these are the changes that the branch has. A diff with the LCA of the target branch.
• The preview diff - the changes that would be apparent if the source is merged into the target.

There is quite a bit of confusion about having two diffs, as the only way the preview diff was ever generated was with an external script.

What we are going to show the user is changing. When the proposal is created, Launchpad will create the diff against the target. This will be shown on the main page where the review diff shows currently. Whenever the tip of the source branch changes, the diff will be regenerated. Users will see an updating diff on the proposal page. We intend to record the diff with the comment to show that a comment may have been made against a different diff.

If a project also wants to update the diffs when the target branch moves, then they will have to run the mad script themselves.

Status Changes

Merge proposals have one status right now. This status tries to show two pieces of information. The current values of the queue_status are:

• Work in Progress
• Needs Review
• Approved
• Rejected
• Queued (you don't see this yet)
• Merge Failed (you don't see this either yet)
• Merged
• Superseded

What we really have here are two distinct states:

• Review status
  • Work in Progress
  • Needs Review
  • Approved
  • Rejected
  • Superseded
• Queue status
  • (some empty value - yet to be determined)
  • Queued
  • Merge Failed
  • Merged

We are also looking at adding another review status: Withdrawn

Recording History

Historical status changes are not currently recorded. Currently if someone has the branch in needs review, then approved, then work in progress, then needs review again, we can't track that. We are going to add a historical table that will record who sets the status and to what.

We are also going to add a column to remember if a user reviewed on behalf of a team or not, and which team.

Workflow Changes

When a new branch is created on launchpad, we can look at the target of the branch (project or package) to see if it looks like the branch is a feature branch (most will be). If we think it is, then we create a "work in progress" merge proposal, but don't send any emails at this stage. This allows us to create moving diffs as the users push changes to launchpad.

If we have chosen the wrong branch, then we allow the user to update the target, like changing lp:launchpad for lp:launchpad/devel. This will cause the diff to be regenerated.

When the user wants the review, they just say "now this needs a review". This will change the state from "work in progress" to "needs review". This will cause the review email to be sent, and start the review process. We could even stop traversal to merge proposals that are in "work in progress" to stop comments being added prematurely.

Ideally, I'd like to show this diff on the branch page itself, so the casual observer of branches could see at a glance what the branch introduces.

This would require us to constrain the merge proposal creation so there could be only one that is active at a time.

Resubmitting

With the moving diff, we are hoping to avoid some unnecessary resubmissions.

Resubmitting will still be an option. We want to keep the conversation shown on the new page while at the same time indicating it was for an earlier submission.

Resubmitting will cause the reviewers to be copied to the new proposal with thier state set to Pending.

Commits shown in the conversion

Commits that happen after the proposal was created will show in this conversation history like bug status updates. The date that Launchpad sees these revisions will be where in the conversation they appear, not when the commit actually happend.

The summary should show the date of the revision, a link to the revision in bazaar.launchpad.net and the commit message.