registry team mailing list archive

[Diogo Matsubara <matsubara@xxxxxxxxxxxxx>]

Diogo Matsubara has proposed merging lp:~matsubara/bzr-pqm/lp-land-update-mp into lp:bzr-pqm.

Requested reviews:
  Bzr-pqm-devel (bzr-pqm-devel)


Hi,

this branch changes the lpland.py command to add the built commit message to the merge proposal. This is done in preparation for https://dev.launchpad.net/Foundations/Proposals/SimplifyMergeMachinery

It also changes the method to get_commit_message to build_commit_message.

This is the same change done for ec2 land here: https://code.launchpad.net/~matsubara/launchpad/ec2-land-update-mp/+merge/40231
-- 
https://code.launchpad.net/~matsubara/bzr-pqm/lp-land-update-mp/+merge/40347
Your team Bzr-pqm-devel is requested to review the proposed merge of lp:~matsubara/bzr-pqm/lp-land-update-mp into lp:bzr-pqm.

=== modified file 'lpland.py'
--- lpland.py	2010-09-09 12:27:00 +0000
+++ lpland.py	2010-11-08 16:54:43 +0000
@@ -179,21 +179,35 @@
         # XXX: JonathanLange 2009-09-24: No unit tests.
         return branch.composePublicURL(scheme="bzr+ssh")
 
-    def get_commit_message(self, commit_text, testfix=False, no_qa=False,
-                           incremental=False, rollback=None):
+    def build_commit_message(self, commit_text, testfix=False, no_qa=False,
+                             incremental=False, rollback=None):
         """Get the Launchpad-style commit message for a merge proposal."""
         reviews = self.get_reviews()
         bugs = self.get_bugs()
 
-        tags = ''.join([
+        tags = [
             get_testfix_clause(testfix),
             get_reviewer_clause(reviews),
             get_bugs_clause(bugs),
             get_qa_clause(bugs, no_qa,
                 incremental, rollback=rollback),
-            ])
-
-        return '%s %s' % (tags, commit_text)
+            ]
+
+        # Make sure we don't add duplicated tags to commit_text.
+        commit_tags = tags[:]
+        for tag in tags:
+            if tag in commit_text:
+                commit_tags.remove(tag)
+
+        if commit_tags:
+            return '%s %s' % (''.join(commit_tags), commit_text)
+        else:
+            return commit_text
+
+    def set_commit_message(self, commit_message):
+        """Set the Launchpad-style commit message for a merge proposal."""
+        self._mp.commit_message = commit_message
+        self._mp.lp_save()
 
 
 class Submitter(object):
@@ -230,8 +244,14 @@
             rollback=None):
         pqm_command = ''.join(submission.to_lines())
         commit_message = mp.commit_message or ''
-        start_message = mp.get_commit_message(commit_message, testfix, no_qa,
-            incremental, rollback=rollback)
+        start_message = mp.build_commit_message(commit_message, testfix,
+            no_qa, incremental, rollback=rollback)
+        try:
+            mp.set_commit_message(start_message)
+        except Exception, e:
+            raise BzrCommandError(
+                "Unable to set the commit message in the merge proposal.\n"
+                "Got: %s" % e)
         message = msgeditor.edit_commit_message(
             'pqm command:\n%s' % pqm_command,
             start_message=start_message).rstrip('\n')

=== modified file 'tests/test_lpland.py'
--- tests/test_lpland.py	2010-09-09 12:27:00 +0000
+++ tests/test_lpland.py	2010-11-08 16:54:43 +0000
@@ -60,6 +60,10 @@
 
     def __init__(self, root=None):
         self._root = root
+        self.commit_message = None
+
+    def lp_save(self):
+        pass
 
 
 class TestBugsClaused(unittest.TestCase):
@@ -206,7 +210,7 @@
         self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
 
         self.assertEqual("[r=foo][ui=none][bug=20] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.",
+            self.mp.build_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
     def test_commit_no_bugs_no_noqa(self):
@@ -217,7 +221,7 @@
         self.mp.get_bugs = FakeMethod([])
         self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
 
-        self.assertRaises(MissingBugsError, self.mp.get_commit_message,
+        self.assertRaises(MissingBugsError, self.mp.build_commit_message,
             testfix, no_qa, incr)
 
     def test_commit_no_bugs_with_noqa(self):
@@ -229,7 +233,7 @@
         self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
 
         self.assertEqual("[r=foo][ui=none][no-qa] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.",
+            self.mp.build_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
     def test_commit_bugs_with_noqa(self):
@@ -242,7 +246,7 @@
 
         self.assertEqual(
             "[r=foo][ui=none][bug=20][no-qa] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.",
+            self.mp.build_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
     def test_commit_bugs_with_incr(self):
@@ -255,7 +259,7 @@
 
         self.assertEqual(
             "[r=foo][ui=none][bug=20][incr] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.",
+            self.mp.build_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
     def test_commit_no_bugs_with_incr(self):
@@ -268,7 +272,7 @@
 
         self.assertEqual(
             "[r=foo][ui=none][bug=20][incr] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.",
+            self.mp.build_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
     def test_commit_with_noqa_and_incr(self):
@@ -281,7 +285,7 @@
 
         self.assertEqual(
             "[r=foo][ui=none][bug=20][no-qa][incr] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.", 
+            self.mp.build_commit_message("Foobaring the sbrubble.", 
                 testfix, no_qa, incr))
 
     def test_commit_with_rollback(self):
@@ -290,8 +294,29 @@
 
         self.assertEqual(
             "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
-            self.mp.get_commit_message("Foobaring the sbrubble.", 
-                rollback=123))
+            self.mp.build_commit_message("Foobaring the sbrubble.", 
+                rollback=123))
+
+    def test_takes_into_account_existing_tags_on_commit_text(self):
+        self.mp.get_bugs = FakeMethod([self.fake_bug])
+        self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
+
+        self.assertEqual(
+            "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
+            self.mp.build_commit_message(
+                "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
+                rollback=123))
+
+
+class TestSetCommitMessage(unittest.TestCase):
+
+    def setUp(self):
+        self.mp = MergeProposal(FakeLPMergeProposal())
+
+    def test_set_commit_message(self):
+        commit_message = "Foobaring the sbrubble."
+        self.mp.set_commit_message(commit_message)
+        self.assertEqual(self.mp._mp.commit_message, commit_message)
 
 
 class TestGetReviewerClause(unittest.TestCase):