Diff for "Code/Review"

Not logged in - Log In / Register

Differences between revisions 6 and 29 (spanning 23 versions)
Revision 6 as of 2008-09-12 16:22:13
Size: 4000
Editor: 92-237-59-186
Comment:
Revision 29 as of 2024-03-19 15:10:02
Size: 8119
Comment: errata: naming
Deletions are marked like this. Additions are marked like this.
Line 1: Line 1:
'''DRAFT: this page is a work in progress. Please [[Feedback|seek further help]] or check the [[TitleIndex|wiki index]] for a complete page on this topic.''' ## page was renamed from Code/Review/Draft
~-[[FrontPage|Launchpad Help]] > [[Code]] > Code review -~
Line 9: Line 10:
That flexibility means you can start work on a project without having to get special permissions to commit code. Of course, if you make changes that you want to see integrated into the project's main line, you need a way of telling the main line's owner that you want to merge. That flexibility means you can start work on a project without having to get special permissions to commit code. Of course, if you make changes that you want to see integrated into the project's main line you need a way of telling the main line's owner that you want to merge.
Line 11: Line 12:
Launchpad and Bazaar make that easy. Bazaar was made to merge: even complex merges are hassle-free. Launchpad helps look after the community process of discussing whether a proposed merge is a good idea. Launchpad and Bazaar make that easy. Bazaar was made to merge: even complex merges can be relatively easy. Launchpad helps look after the community process of discussing whether a proposed merge is a good idea.

See also this description [[Code/Review/Relationships|Code review relationships]].
Line 24: Line 27:
<<Anchor(EmailInterface)>>
== Email interface ==

You can also propose a merge by email by sending a [[http://doc.bazaar.canonical.com/bzr.dev/en/user-guide/sending_changes.html|merge directive]] to `merge@code.launchpad.net`.

In your email, you can ask for particular people to do the review by adding lines that say:
 * `reviewer <name>` where `<name>` is the Launchpad user name or email address of the reviewer.

You can ask for as many reviewers as you like. Note that you '''must''' start the command line with a space or your command will not be recognized.

== The flow of this process ==

Understanding how the Code Review process works may be a little tricky to get the hang of, but once you have got the hang of it, then it's really easy. If you're new to this process, this is how it works:

 * You submit your request to merge your branch into the main one (or whichever target branch interests you)
 * Wait for one of the target branch's reviewers to review your code, they'll either:
  * Accept your code, and it will be merged.
  * Modify your code slightly, then merge it.
  * Tell you what to do with a [[Comments|comment]], marking your review as "Needs Fixing".
   * If you're happy to, you do make the changes requested and push them into your branch. Then [[Comments|comment]] back on your proposal that you have done that. You should now await further action.
  * Reject the changes. At this point, you can either try again discuss with the branch owner how to make your changes acceptable or continue work in your own branch.

== Managing the Merge Proposal ==

A merge proposal can be in one of several statuses, each representing the proposal's current state in the review lifecycle, as described above.

Several rules apply to the transitions in-between these statuses. Transitioning to '''Approved''' or '''Rejected''' from '''Work in progress''' or '''Needs review''' needs the user to be a valid reviewer (branch owner and designated reviewers).
Line 26: Line 57:
Once you've proposed a merge, anyone who has a Launchpad account can comment and vote on the proposal. Taking part in a code review is virtually hassle-free: Once you've proposed a merge, anyone who has a Launchpad account can [[Comments|comment]] and vote on the proposal. Taking part in a code review is virtually hassle-free:
Line 33: Line 64:
As a subscriber to or participant in the conversation, you'll receive email updates as the code progresses. Just as with the bug tracker, you can take part in the review both by visiting Launchpad's web interface and by replying to any of the emails you receive. In effect, each code review becomes an ad-hoc mailing list that exists for the lifetime of the review. As a subscriber to the branch or a participant in the code review, you'll receive email updates each time someone adds a vote or message to the review. Just as with the bug tracker, you can take part in the review both by visiting Launchpad's web interface and by replying to any of the emails you receive. In effect, each code review becomes an ad-hoc mailing list that exists for the lifetime of the review.
Line 41: Line 72:
Using the code review email interface is straightforward. Reply to an email from the code review and your comment is added to the discussion in Launchpad. If you want to vote by email, leave a space at the start of the line and then one of the following commands: Using the code review email interface is straightforward. Reply to an email from the code review and your [[Comments|comment]] is added to the discussion in Launchpad.
Line 43: Line 74:
 * vote approve
 * vote disapprove
 * vote abstain
If you want to include commands to modify the approval or status of the review you must either sign your email with your GPG key that is associated with your Launchpad account, or send through a trusted DKIM sender such as GMail.
Line 47: Line 76:
So, if you wanted to vote ''disapprove'', add a tag of ''UI'' and also leave a comment, you'd write: Here is the list of available commands:

Note that you '''must''' start the command line with a space or your command will not be recognized.

 * `review` - inform the developer what you think of their changes
   * `review approve` - approve the change ([[http://www.python.org/dev/peps/pep-0010/|alias]]: `+1`)
   * `review disapprove` - disapprove of the change ([[http://www.python.org/dev/peps/pep-0010/|alias]]: `-1`)
   * `review abstain` - abstain from deciding ([[http://www.python.org/dev/peps/pep-0010/|alias]]: `-0` and `+0`)
   * `review resubmit` - tell the developer to rework the change
   * `review needs-fixing` - tell the developer that a few things need improvement (alias: `needsfixing` and `needs_fixing`)
   * `review needs-info` - tell the developer that you need more information (alias: `needsinfo` and `needs-information` and `needsinformation`)
 * `merge` - either approve or reject the proposed change
   * `merge approved` - approve of the merge proposal (alias: `approve`)
   * `merge rejected` - reject the merge proposal (alias: `reject`)
 * `reviewer` - add a new reviewer to the merge proposal
   * `reviewer <name>` where `<name>` is the Launchpad user name or email address of the new reviewer.
 * `vote` (deprecated: use `review`)
 * `status` (deprecated: use `merge`)

You can combine commands, so if you wanted to vote ''disapprove'', add a tag of ''UI'', leave a [[Comments|comment]], and reject the merge proposal, you'd write:
Line 50: Line 98:
This is a sensisble change but I find the user interface confusing. This is a sensible change but I find the user interface confusing.
Line 52: Line 100:
 vote disapprove UI  review disapprove UI
 merge rejected
Line 54: Line 103:

If you simply want to approve the proposal, using `merge approved` will also implicitly add an equivalent `review approve` unless you specify a `review` command separately.


= Making the merge =

Once you're ready to merge another branch into yours, follow the instructions on the page, either for bzr or for git respectively.
Line 58: Line 114:

||<tablestyle="width: 100%;"> ~-[[Code/TeamBranches|< Team branches]] -~ ||<style="text-align: right;"> ~- [[Code/BugAndBlueprintLinks|Linking branches to bug reports and blueprints >]] -~ ||

Launchpad Help > Code > Code review

Overview

Thanks to Bazaar's distributed model, you can get full access to the code of any branch hosted in Launchpad, with version control and history, right on your own machine.

That flexibility means you can start work on a project without having to get special permissions to commit code. Of course, if you make changes that you want to see integrated into the project's main line you need a way of telling the main line's owner that you want to merge.

Launchpad and Bazaar make that easy. Bazaar was made to merge: even complex merges can be relatively easy. Launchpad helps look after the community process of discussing whether a proposed merge is a good idea.

See also this description Code review relationships.

Proposing a merge

proposed-merges.png

Proposed merges

When you've come to a stage in your development where you're ready to merge your code into another branch registered against the project - such as its main line - you can make a public merge proposal.

To do so, visit your branch's overview page, click Propose for merging into another branch, then follow the on-screen instructions.

Now, Launchpad will notify the proposed target branch's owner of your proposal. Anyone viewing the overview page for your branch, or the target branch, will also see a link to view the details of your proposal.

Email interface

You can also propose a merge by email by sending a merge directive to merge@code.launchpad.net.

In your email, you can ask for particular people to do the review by adding lines that say:

  • reviewer <name> where <name> is the Launchpad user name or email address of the reviewer.

You can ask for as many reviewers as you like. Note that you must start the command line with a space or your command will not be recognized.

The flow of this process

Understanding how the Code Review process works may be a little tricky to get the hang of, but once you have got the hang of it, then it's really easy. If you're new to this process, this is how it works:

  • You submit your request to merge your branch into the main one (or whichever target branch interests you)
  • Wait for one of the target branch's reviewers to review your code, they'll either:
    • Accept your code, and it will be merged.
    • Modify your code slightly, then merge it.
    • Tell you what to do with a comment, marking your review as "Needs Fixing".

      • If you're happy to, you do make the changes requested and push them into your branch. Then comment back on your proposal that you have done that. You should now await further action.

    • Reject the changes. At this point, you can either try again discuss with the branch owner how to make your changes acceptable or continue work in your own branch.

Managing the Merge Proposal

A merge proposal can be in one of several statuses, each representing the proposal's current state in the review lifecycle, as described above.

Several rules apply to the transitions in-between these statuses. Transitioning to Approved or Rejected from Work in progress or Needs review needs the user to be a valid reviewer (branch owner and designated reviewers).

Code review

Once you've proposed a merge, anyone who has a Launchpad account can comment and vote on the proposal. Taking part in a code review is virtually hassle-free:

  • no barriers: anyone can take part, so long as they have a Launchpad account

  • convenient: you can get updates and contribute using email, as well as the web interface.

Each review consists of votes - approve, abstain or disapprove - and a threaded conversation much as you might find on a web forum or a mailing list. If you're already familiar with Launchpad Bugs, you'll be right at home with code reviews.

As a subscriber to the branch or a participant in the code review, you'll receive email updates each time someone adds a vote or message to the review. Just as with the bug tracker, you can take part in the review both by visiting Launchpad's web interface and by replying to any of the emails you receive. In effect, each code review becomes an ad-hoc mailing list that exists for the lifetime of the review.

If you want to make your vote specific to one aspect of the proposed merge, you can add a tag. For example: if you wanted to vote disapprove, based on the user interface, you could add a tag of ui.

For an example of a review, take a look at a code review in the Storm project.

Email interface

Using the code review email interface is straightforward. Reply to an email from the code review and your comment is added to the discussion in Launchpad.

If you want to include commands to modify the approval or status of the review you must either sign your email with your GPG key that is associated with your Launchpad account, or send through a trusted DKIM sender such as GMail.

Here is the list of available commands:

Note that you must start the command line with a space or your command will not be recognized.

  • review - inform the developer what you think of their changes

    • review approve - approve the change (alias: +1)

    • review disapprove - disapprove of the change (alias: -1)

    • review abstain - abstain from deciding (alias: -0 and +0)

    • review resubmit - tell the developer to rework the change

    • review needs-fixing - tell the developer that a few things need improvement (alias: needsfixing and needs_fixing)

    • review needs-info - tell the developer that you need more information (alias: needsinfo and needs-information and needsinformation)

  • merge - either approve or reject the proposed change

    • merge approved - approve of the merge proposal (alias: approve)

    • merge rejected - reject the merge proposal (alias: reject)

  • reviewer - add a new reviewer to the merge proposal

    • reviewer <name> where <name> is the Launchpad user name or email address of the new reviewer.

  • vote (deprecated: use review)

  • status (deprecated: use merge)

You can combine commands, so if you wanted to vote disapprove, add a tag of UI, leave a comment, and reject the merge proposal, you'd write:

This is a sensible change but I find the user interface confusing.

 review disapprove UI
 merge rejected

If you simply want to approve the proposal, using merge approved will also implicitly add an equivalent review approve unless you specify a review command separately.

Making the merge

Once you're ready to merge another branch into yours, follow the instructions on the page, either for bzr or for git respectively.

Next step

You can pick choose which parts of Launchpad you want to use. However, when you use different parts of Launchpad together you can make them work together. Let's look at how you can link bug report and blueprints to branches of code.

< Team branches

Linking branches to bug reports and blueprints >

Code/Review (last edited 2024-03-19 15:10:02 by ruinedyourlife)