Merge lp:~abentley/bzr/lpsubmit into lp:bzr

[Aaron Bentley]

Add lp-submit command.

[Aaron Bentley (abentley)]

Hi all,

This branch introduces the lp-submit command, originally from the bzr-pipeline
plugin.

It includes two hooks, one for determining the prerequisite branch, and one
for determining the message body. The pipeline plugin will use the first
to supply the previous pipe as the prerequisite branch. The lpreview_body
plugin will use the second to supply a body following Launchpad policies.

This merge proposal was, itself, submitted using the lp-submit command.

[John A Meinel (jameinel)]

I didn't focus too concretely on this, since it is part of the Launchpad plugin. (So the review and coding standards are looser than for bzrlib code.)

Some comments

1) There doesn't seem to be a way to supply the initial merge comment from the command line, as '-m' sets the commit message instead. *My* initial expectation was that 'bzr lp-submit -m ...' would have set the initial comment (and not popped up an editor). Though probably that is because *bzr* doesn't use the Commit Message field in Launchpad for anything. (We set it at pqm-submit time.)

However, using -m is probably fine, as it does mimic pqm-submit -m and commit -m for a system like Tarmac.

I would think you'd want a similar --comment="And here is my summary comment" flag, though.

2) The code seems fine. No tests, but it generally needs to be tested against Launchpad anyway.

review: Approve

[Martin Pool (mbp)]

This would be good to add.

Needs news.

Needs a mention in the user guide.

The help for cmd_lp_submit doesn't really explain what this does or why you would want to use it. It looks like creates a merge proposal or requests a merge - that's what the Launchpad UI calls it but this seems to have no connection.

review: Needs Fixing

[Martin Pool (mbp)]

It also ought to document, eg by way of an example, or in the option help, what the reviewers option does and its syntax.

Would it be better to just call this "lp-propose" rather than "lp-submit"? To me "submit" is equally likely to mean 'submit to pqm' after you're done.

lp_submit.py needs a copyright statement.

415 + except restful_errors.HTTPError, e:
416 + for line in e.content.splitlines():
417 + if line.startswith('Traceback (most recent call last):'):
418 + break
419 + print line

This should probably turn it into an error raised to the caller?

The following are optional or follow-ons:

It would be nice to test it at least down to the layer of the lp api, either through function calls or a --dry-run option.

some of the lp code could be in common parts, but it can be refactored later.

review: Needs Fixing

[Martin Pool (mbp)]

Oh, also, I suppose this should say that it will then open the mp in a browser.

As a follow-on one might want the option to turn this off.

[Martin Pool (mbp)]

I tested this interactively and it's really very nice, cleaning up a snag in the process.

It takes a while to get going so it would be good if it said just what it is doing.

It would also be nice if the message editor told me about the revisions newly added and perhaps showed their diff - in fact that would be a bit of a killer feature over Launchpad.

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John A Meinel wrote:
> *My* initial expectation was that 'bzr lp-submit -m ...' would have set the initial comment (and not popped up an editor). Though probably that is because *bzr* doesn't use the Commit Message field in Launchpad for anything. (We set it at pqm-submit time.)

The new lp-land command in bzr-pqm does use the commit message field.

> I would think you'd want a similar --comment="And here is my summary comment" flag, though.

So far, I haven't wanted that. In fact, one of the major advantages of
the command is that, like "bzr send" it allows a template proposal
message to be supplied. Launchpad's hook even includes lint output.
Providing an initial merge comment at the command line loses that
advantage. So I'm not sure it's worthwhile, though of course I wouldn't
block a reasonable implementation of it, if someone wanted to do that.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktyMlYACgkQ0F+nu1YWqI1Z+ACfRk1K5pPQ2F9JTpafxeGugR96
gKEAnA6F6BspWrcRIIx4yQKcb5HrLOZF
=Lzkm
-----END PGP SIGNATURE-----

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Review: Needs Fixing
> It also ought to document, eg by way of an example, or in the option help, what the reviewers option does and its syntax.

Done.

> Would it be better to just call this "lp-propose" rather than "lp-submit"?

I've gone for maximum clarity and called it lp-propose-merge, with
lp-propose and lp-submit as aliases.

> To me "submit" is equally likely to mean 'submit to pqm' after you're done.

To me, the effect of submitting something should depend on what you're
submitting it to.

> lp_submit.py needs a copyright statement.

Done.

> 415 + except restful_errors.HTTPError, e:
> 416 + for line in e.content.splitlines():
> 417 + if line.startswith('Traceback (most recent call last):'):
> 418 + break
> 419 + print line
>
> This should probably turn it into an error raised to the caller?

Done.

> The following are optional or follow-ons:

I'll leave those for later.

> Oh, also, I suppose this should say that it will then open the mp in a browser.

Done.

> As a follow-on one might want the option to turn this off.

I'll leave that for later. (For the record, I *always* want it opened
in a browser, because once it's been proposed, I need to communicate
with potential reviewers about it.)

> It takes a while to get going so it would be good if it said just
> what it is doing.

I suppose. The sad thing is, AFAICT, the delay is the time it takes to
log into Launchpad.

> It would also be nice if the message editor told me about the
> revisions newly added and perhaps showed their diff - in fact that would
> be a bit of a killer feature over Launchpad.

Yes, that could be nice, but I prefer to see the diff in a terminal.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktyNkUACgkQ0F+nu1YWqI0XEQCfUfLCZ7Ct2owzZdzTs+VmJBsL
yKEAn0W8WD4vZVLVbf7RjdDI2NhW3ZwA
=XbZJ
-----END PGP SIGNATURE-----

[Aaron Bentley (abentley)]

About the user guide: it's in the command listing; isn't that enough? If not, where exactly do you want it to go?

[Martin Pool (mbp)]

I used this for all my proposals yesterday and it was really nice - thanks!

> > Would it be better to just call this "lp-propose" rather than "lp-submit"?
>
> I've gone for maximum clarity and called it lp-propose-merge, with
> lp-propose and lp-submit as aliases.

That's good, thanks.

> > To me "submit" is equally likely to mean 'submit to pqm' after you're done.
>
> To me, the effect of submitting something should depend on what you're
> submitting it to.

I don't understand - we could use 'submit' to mean 'commit' when you're submitting from a tree to a branch, but calling that 'submit' would just cause confusion.

I think having multiple synonyms for commands (that aren't abbreviations) was a mistake and just causes confusion about whether they're the same or not.

So unless there's some reason I'm not aware of for 'lp-submit' please remove that alias.

> > It takes a while to get going so it would be good if it said just
> > what it is doing.
>
> I suppose. The sad thing is, AFAICT, the delay is the time it takes to
> log into Launchpad.

(In passing https://bugs.edge.launchpad.net/launchpadlib/+bug/520219)

review: Needs Fixing

[Martin Pool (mbp)]

> About the user guide: it's in the command listing; isn't that enough? If not,
> where exactly do you want it to go?

I think it should go into 'sharing with peers' in the user guide. Just a sentence or two making people aware that it exists is enough to let them chase the link to the help or user reference.

Also we should add it to help.l.n

[Andrew Bennetts (spiv)]

Martin Pool wrote:
[...]
> I think having multiple synonyms for commands (that aren't abbreviations) was
> a mistake and just causes confusion about whether they're the same or not.

Agreed — and I believe we've started taking steps to remove some of the
synonyms we already have, so adding more (even in a plugin) seems like a
step backwards.

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
>>> To me "submit" is equally likely to mean 'submit to pqm' after you're done.
>> To me, the effect of submitting something should depend on what you're
>> submitting it to.
>
> I don't understand - we could use 'submit' to mean 'commit' when you're submitting from a tree to a branch, but calling that 'submit' would just cause confusion.

I mean that 'submit' as a verb doesn't really suggest an outcome. If
you submit to a magazine, you may get an article published. If you
submit a premise, you may win an argument. If you submit to a PQM, you
may get your branch merged and committed. If you submit to Launchpad,
you may get your code reviewed. Note that whatever we call the command,
we will default to the "submit" branch as the target.

> I think having multiple synonyms for commands (that aren't abbreviations) was a mistake and just causes confusion about whether they're the same or not.
>
> So unless there's some reason I'm not aware of for 'lp-submit' please remove that alias.

The lp-submit command is well-established in the "bzr-pipeline" plugin.
 The alias is to ease the transition for people used to using that name.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktzpt0ACgkQ0F+nu1YWqI1CrACdGPjcjfLFwchodYLazuwhSX8p
ELsAnj+2L5G6jncoECjYG1DmPtjjmZIh
=cT4L
-----END PGP SIGNATURE-----

[Martin Pool (mbp)]

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Martin Pool wrote:
> >>> To me "submit" is equally likely to mean 'submit to pqm' after you're
> done.
> >> To me, the effect of submitting something should depend on what you're
> >> submitting it to.
> >
> > I don't understand - we could use 'submit' to mean 'commit' when you're
> submitting from a tree to a branch, but calling that 'submit' would just cause
> confusion.
>
> I mean that 'submit' as a verb doesn't really suggest an outcome. If
> you submit to a magazine, you may get an article published. If you
> submit a premise, you may win an argument. If you submit to a PQM, you
> may get your branch merged and committed. If you submit to Launchpad,
> you may get your code reviewed. Note that whatever we call the command,
> we will default to the "submit" branch as the target.

But this is just to say that submit is a very general verb, whereas if we're going to use it as the name of a command then having a more specific one is good.

Anyhow, changing the primary name and keeping the alias for compatibility is ok.

I would like to see the actual code changed to be 'propose' not 'submit'.

I would change canonical_url to something like web_url and make the replacement be

  launchpad_object.self_link.replace('api.', '', 1).replace('/beta/', '/', 1)

to make it more robust, and maybe add a comment pointing to https://bugs.edge.launchpad.net/launchpadlib/+bug/316694

aside from that, good to go, thanks.

review: Needs Fixing

[lp:~abentley/bzr/lpsubmit]

4986. By Aaron Bentley on 2010-02-18

Merged bzr.dev into lpsubmit.

4987. By Aaron Bentley on 2010-02-18

Fix divergence check.

4988. By Aaron Bentley on 2010-02-18

Rename submit to propose everywhere.

4989. By Aaron Bentley on 2010-02-18

Fix exception handling.

=== modified file 'NEWS'
--- NEWS	2010-02-18 04:04:19 +0000
+++ NEWS	2010-02-18 04:28:14 +0000
@@ -49,6 +49,9 @@
   as resolved is still accessible via the ``--done`` default action.
   (Vincent Ladeuil)
 
+* Merges can be proposed on Launchpad with the new lp-propose-merge command.
+  (Aaron Bentley, Jonathan Lange)
+
 Bug Fixes
 *********
 

=== modified file 'bzrlib/plugins/launchpad/__init__.py'
--- bzrlib/plugins/launchpad/__init__.py	2009-12-17 04:35:06 +0000
+++ bzrlib/plugins/launchpad/__init__.py	2010-02-18 04:28:14 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 - 2008 Canonical Ltd
+# Copyright (C) 2006 - 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -32,7 +32,11 @@
     )
 """)
 
-from bzrlib.commands import Command, Option, register_command
+from bzrlib import bzrdir
+from bzrlib.commands import (
+        Command,
+        register_command,
+)
 from bzrlib.directory_service import directories
 from bzrlib.errors import (
     BzrCommandError,
@@ -42,6 +46,10 @@
     NotBranchError,
     )
 from bzrlib.help_topics import topic_registry
+from bzrlib.option import (
+        Option,
+        ListOption,
+)
 
 
 class cmd_register_branch(Command):
@@ -277,6 +285,67 @@
 register_command(cmd_launchpad_mirror)
 
 
+class cmd_lp_propose_merge(Command):
+    """Propose merging a branch on Launchpad.
+
+    This will open your usual editor to provide the initial comment.  When it
+    has created the proposal, it will open it in your default web browser.
+
+    The branch will be proposed to merge into SUBMIT_BRANCH.  If SUBMIT_BRANCH
+    is not supplied, the remembered submit branch will be used.  If no submit
+    branch is remembered, the development focus will be used.
+
+    By default, the SUBMIT_BRANCH's review team will be requested to review
+    the merge proposal.  This can be overriden by specifying --review (-R).
+    The parameter the launchpad account name of the desired reviewer.  This
+    may optionally be followed by '=' and the review type.  For example:
+
+      bzr lp-propose-merge --review jrandom --review review-team=qa
+
+    This will propose a merge,  request "jrandom" to perform a review of
+    unspecified type, and request "review-team" to perform a "qa" review.
+    """
+
+    takes_options = [Option('staging',
+                            help='Propose the merge on staging.'),
+                     Option('message', short_name='m', type=unicode,
+                            help='Commit message.'),
+                     ListOption('review', short_name='R', type=unicode,
+                            help='Requested reviewer and optional type.')]
+
+    takes_args = ['submit_branch?']
+
+    aliases = ['lp-submit', 'lp-propose']
+
+    def run(self, submit_branch=None, review=None, staging=False,
+            message=None):
+        from bzrlib.plugins.launchpad import lp_propose
+        tree, branch, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(
+            '.')
+        if review is None:
+            reviews = None
+        else:
+            reviews = []
+            for review in review:
+                if '=' in review:
+                    reviews.append(review.split('=', 2))
+                else:
+                    reviews.append((review, ''))
+            if submit_branch is None:
+                submit_branch = branch.get_submit_branch()
+        if submit_branch is None:
+            target = None
+        else:
+            target = _mod_branch.Branch.open(submit_branch)
+        proposer = lp_propose.Proposer(tree, branch, target, message,
+                                       reviews, staging)
+        proposer.check_proposal()
+        proposer.create_proposal()
+
+
+register_command(cmd_lp_propose_merge)
+
+
 def _register_directory():
     directories.register_lazy('lp:', 'bzrlib.plugins.launchpad.lp_directory',
                               'LaunchpadDirectory',

=== modified file 'bzrlib/plugins/launchpad/lp_api.py'
--- bzrlib/plugins/launchpad/lp_api.py	2009-12-17 03:40:41 +0000
+++ bzrlib/plugins/launchpad/lp_api.py	2010-02-18 04:28:14 +0000
@@ -22,11 +22,15 @@
 
 
 import os
+import re
 
 from bzrlib import (
+    branch,
     config,
     errors,
     osutils,
+    trace,
+    transport,
     )
 from bzrlib.plugins.launchpad.lp_registration import (
     InvalidLaunchpadInstance,
@@ -112,6 +116,149 @@
     return launchpad
 
 
+class LaunchpadBranch(object):
+    """Provide bzr and lp API access to a Launchpad branch."""
+
+    def __init__(self, lp_branch, bzr_url, bzr_branch=None, check_update=True):
+        """Constructor.
+
+        :param lp_branch: The Launchpad branch.
+        :param bzr_url: The URL of the Bazaar branch.
+        :param bzr_branch: An instance of the Bazaar branch.
+        """
+        self.bzr_url = bzr_url
+        self._bzr = bzr_branch
+        self._push_bzr = None
+        self._check_update = check_update
+        self.lp = lp_branch
+
+    @property
+    def bzr(self):
+        """Return the bzr branch for this branch."""
+        if self._bzr is None:
+            self._bzr = branch.Branch.open(self.bzr_url)
+        return self._bzr
+
+    @property
+    def push_bzr(self):
+        """Return the push branch for this branch."""
+        if self._push_bzr is None:
+            self._push_bzr = branch.Branch.open(self.lp.bzr_identity)
+        return self._push_bzr
+
+    @staticmethod
+    def plausible_launchpad_url(url):
+        """Is 'url' something that could conceivably be pushed to LP?
+
+        :param url: A URL that may refer to a Launchpad branch.
+        :return: A boolean.
+        """
+        if url is None:
+            return False
+        if url.startswith('lp:'):
+            return True
+        regex = re.compile('([a-z]*\+)*(bzr\+ssh|http)'
+                           '://bazaar.*.launchpad.net')
+        return bool(regex.match(url))
+
+    @staticmethod
+    def candidate_urls(bzr_branch):
+        """Iterate through related URLs that might be Launchpad URLs.
+
+        :param bzr_branch: A Bazaar branch to find URLs from.
+        :return: a generator of URL strings.
+        """
+        url = bzr_branch.get_public_branch()
+        if url is not None:
+            yield url
+        url = bzr_branch.get_push_location()
+        if url is not None:
+            yield url
+        yield bzr_branch.base
+
+    @staticmethod
+    def tweak_url(url, launchpad):
+        """Adjust a URL to work with staging, if needed."""
+        if str(launchpad._root_uri) != STAGING_SERVICE_ROOT:
+            return url
+        if url is None:
+            return None
+        return url.replace('bazaar.launchpad.net',
+                           'bazaar.staging.launchpad.net')
+
+    @classmethod
+    def from_bzr(cls, launchpad, bzr_branch):
+        """Find a Launchpad branch from a bzr branch."""
+        check_update = True
+        for url in cls.candidate_urls(bzr_branch):
+            url = cls.tweak_url(url, launchpad)
+            if not cls.plausible_launchpad_url(url):
+                continue
+            lp_branch = launchpad.branches.getByUrl(url=url)
+            if lp_branch is not None:
+                break
+        else:
+            lp_branch = cls.create_now(launchpad, bzr_branch)
+            check_update = False
+        return cls(lp_branch, bzr_branch.base, bzr_branch, check_update)
+
+    @classmethod
+    def create_now(cls, launchpad, bzr_branch):
+        """Create a Bazaar branch on Launchpad for the supplied branch."""
+        url = cls.tweak_url(bzr_branch.get_push_location(), launchpad)
+        if not cls.plausible_launchpad_url(url):
+            raise errors.BzrError('%s is not registered on Launchpad' %
+                                  bzr_branch.base)
+        bzr_branch.create_clone_on_transport(transport.get_transport(url))
+        lp_branch = launchpad.branches.getByUrl(url=url)
+        if lp_branch is None:
+            raise errors.BzrError('%s is not registered on Launchpad' % url)
+        return lp_branch
+
+    def get_dev_focus(self):
+        """Return the 'LaunchpadBranch' for the dev focus of this one."""
+        lp_branch = self.lp
+        if lp_branch.project is None:
+            raise errors.BzrError('%s has no product.' %
+                                  lp_branch.bzr_identity)
+        dev_focus = lp_branch.project.development_focus.branch
+        if dev_focus is None:
+            raise errors.BzrError('%s has no development focus.' %
+                                  lp_branch.bzr_identity)
+        return LaunchpadBranch(dev_focus, dev_focus.bzr_identity)
+
+    def update_lp(self):
+        """Update the Launchpad copy of this branch."""
+        if not self._check_update:
+            return
+        self.bzr.lock_read()
+        try:
+            if self.lp.last_scanned_id is not None:
+                if self.bzr.last_revision() == self.lp.last_scanned_id:
+                    trace.note('%s is already up-to-date.' %
+                               self.lp.bzr_identity)
+                    return
+                graph = self.bzr.repository.get_graph()
+                if not graph.is_ancestor(self.lp.last_scanned_id,
+                                         self.bzr.last_revision()):
+                    raise errors.DivergedBranches(self.bzr, self.push_bzr)
+                trace.note('Pushing to %s' % self.lp.bzr_identity)
+            self.bzr.push(self.push_bzr)
+        finally:
+            self.bzr.unlock()
+
+    def find_lca_tree(self, other):
+        """Find the revision tree for the LCA of this branch and other.
+
+        :param other: Another LaunchpadBranch
+        :return: The RevisionTree of the LCA of this branch and other.
+        """
+        graph = self.bzr.repository.get_graph(other.bzr.repository)
+        lca = graph.find_unique_lca(self.bzr.last_revision(),
+                                    other.bzr.last_revision())
+        return self.bzr.repository.revision_tree(lca)
+
+
 def load_branch(launchpad, branch):
     """Return the launchpadlib Branch object corresponding to 'branch'.
 

=== added file 'bzrlib/plugins/launchpad/lp_propose.py'
--- bzrlib/plugins/launchpad/lp_propose.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/launchpad/lp_propose.py	2010-02-18 04:28:14 +0000
@@ -0,0 +1,206 @@
+# Copyright (C) 2009, 2010 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+
+import webbrowser
+
+from bzrlib import (
+    errors,
+    msgeditor,
+)
+from bzrlib.hooks import HookPoint, Hooks
+from bzrlib.plugins.launchpad import (
+    lp_api,
+    lp_registration,
+)
+
+from lazr.restfulclient import errors as restful_errors
+
+
+class ProposeMergeHooks(Hooks):
+    """Hooks for proposing a merge on Launchpad."""
+
+    def __init__(self):
+        Hooks.__init__(self)
+        self.create_hook(
+            HookPoint(
+                'get_prerequisite',
+                "Return the prerequisite branch for proposing as merge.",
+                (2, 1), None),
+        )
+        self.create_hook(
+            HookPoint(
+                'merge_proposal_body',
+                "Return an initial body for the merge proposal message.",
+                (2, 1), None),
+        )
+
+
+class Proposer(object):
+
+    hooks = ProposeMergeHooks()
+
+    def __init__(self, tree, source_branch, target_branch, message, reviews,
+                 staging=False):
+        """Constructor.
+
+        :param tree: The working tree for the source branch.
+        :param source_branch: The branch to propose for merging.
+        :param target_branch: The branch to merge into.
+        :param message: The commit message to use.  (May be None.)
+        :param reviews: A list of tuples of reviewer, review type.
+        :param staging: If True, propose the merge against staging instead of
+            production.
+        """
+        self.tree = tree
+        if staging:
+            lp_instance = 'staging'
+        else:
+            lp_instance = 'edge'
+        service = lp_registration.LaunchpadService(lp_instance=lp_instance)
+        self.launchpad = lp_api.login(service)
+        self.source_branch = lp_api.LaunchpadBranch.from_bzr(
+            self.launchpad, source_branch)
+        if target_branch is None:
+            self.target_branch = self.source_branch.get_dev_focus()
+        else:
+            self.target_branch = lp_api.LaunchpadBranch.from_bzr(
+                self.launchpad, target_branch)
+        self.commit_message = message
+        if reviews == []:
+            target_reviewer = self.target_branch.lp.reviewer
+            if target_reviewer is None:
+                raise errors.BzrCommandError('No reviewer specified')
+            self.reviews = [(target_reviewer, '')]
+        else:
+            self.reviews = [(self.launchpad.people[reviewer], review_type)
+                            for reviewer, review_type in
+                            reviews]
+
+    def get_comment(self, prerequisite_branch):
+        """Determine the initial comment for the merge proposal."""
+        info = ["Source: %s\n" % self.source_branch.lp.bzr_identity]
+        info.append("Target: %s\n" % self.target_branch.lp.bzr_identity)
+        if prerequisite_branch is not None:
+            info.append("Prereq: %s\n" % prerequisite_branch.lp.bzr_identity)
+        for rdata in self.reviews:
+            uniquename = "%s (%s)" % (rdata[0].display_name, rdata[0].name)
+            info.append('Reviewer: %s, type "%s"\n' % (uniquename, rdata[1]))
+        self.source_branch.bzr.lock_read()
+        try:
+            self.target_branch.bzr.lock_read()
+            try:
+                body = self.get_initial_body()
+            finally:
+                self.target_branch.bzr.unlock()
+        finally:
+            self.source_branch.bzr.unlock()
+        initial_comment = msgeditor.edit_commit_message(''.join(info),
+                                                        start_message=body)
+        return initial_comment.strip().encode('utf-8')
+
+    def get_initial_body(self):
+        """Get a body for the proposal for the user to modify.
+
+        :return: a str or None.
+        """
+        def list_modified_files():
+            lca_tree = self.source_branch.find_lca_tree(
+                self.target_branch)
+            source_tree = self.source_branch.bzr.basis_tree()
+            files = modified_files(lca_tree, source_tree)
+            return list(files)
+        target_loc = ('bzr+ssh://bazaar.launchpad.net/%s' %
+                       self.target_branch.lp.unique_name)
+        body = None
+        for hook in self.hooks['merge_proposal_body']:
+            body = hook({
+                'tree': self.tree,
+                'target_branch': target_loc,
+                'modified_files_callback': list_modified_files,
+                'old_body': body,
+            })
+        return body
+
+    def check_proposal(self):
+        """Check that the submission is sensible."""
+        if self.source_branch.lp.self_link == self.target_branch.lp.self_link:
+            raise errors.BzrCommandError(
+                'Source and target branches must be different.')
+        for mp in self.source_branch.lp.landing_targets:
+            if mp.queue_status in ('Merged', 'Rejected'):
+                continue
+            if mp.target_branch.self_link == self.target_branch.lp.self_link:
+                raise errors.BzrCommandError(
+                    'There is already a branch merge proposal: %s' %
+                    canonical_url(mp))
+
+    def _get_prerequisite_branch(self):
+        hooks = self.hooks['get_prerequisite']
+        prerequisite_branch = None
+        for hook in hooks:
+            prerequisite_branch = hook(
+                {'launchpad': self.launchpad,
+                 'source_branch': self.source_branch,
+                 'target_branch': self.target_branch,
+                 'prerequisite_branch': prerequisite_branch})
+        return prerequisite_branch
+
+    def create_proposal(self):
+        """Perform the submission."""
+        prerequisite_branch = self._get_prerequisite_branch()
+        if prerequisite_branch is None:
+            prereq = None
+        else:
+            prereq = prerequisite_branch.lp
+            prerequisite_branch.update_lp()
+        self.source_branch.update_lp()
+        reviewers = []
+        review_types = []
+        for reviewer, review_type in self.reviews:
+            review_types.append(review_type)
+            reviewers.append(reviewer.self_link)
+        initial_comment = self.get_comment(prerequisite_branch)
+        try:
+            mp = self.source_branch.lp.createMergeProposal(
+                target_branch=self.target_branch.lp,
+                prerequisite_branch=prereq,
+                initial_comment=initial_comment,
+                commit_message=self.commit_message, reviewers=reviewers,
+                review_types=review_types)
+        except restful_errors.HTTPError, e:
+            error_lines = []
+            for line in e.content.splitlines():
+                if line.startswith('Traceback (most recent call last):'):
+                    break
+                error_lines.append(line)
+            raise Exception(''.join(error_lines))
+        else:
+            webbrowser.open(canonical_url(mp))
+
+
+def modified_files(old_tree, new_tree):
+    """Return a list of paths in the new tree with modified contents."""
+    for f, (op, path), c, v, p, n, (ok, k), e in new_tree.iter_changes(
+        old_tree):
+        if c and k == 'file':
+            yield str(path)
+
+
+def canonical_url(object):
+    """Return the canonical URL for a branch."""
+    url = object.self_link.replace('https://api.', 'https://code.')
+    return url.replace('/beta/', '/')