Merge lp://staging/~oddbloke/pqm/merge-details into lp://staging/pqm

I'd like to propose against my integration branch, but until I land the scanner fix I have I don't want the status of the source branch to be set as merged.

=== modified file 'pqm/__init__.py'
--- pqm/__init__.py 2008-09-29 08:57:02 +0000
+++ pqm/__init__.py 2008-11-24 20:09:58 +0000
@@ -233,6 +233,9 @@
     def make_local_dir(self, sender, branch, output_dir):
         raise PQMTlaFailure(sender, 'Unsupported operation')

+ def latest_revision_info(self, sender, branch):
+ raise PQMTlaFailure(sender, 'Unsupported operation')
+

 class ArchHandler(VCSHandler):

@@ -465,6 +468,11 @@
         branch = Branch.open(branch_spec)
         branch.create_checkout(output_dir, lightweight=True)

+ def latest_revision_info(self, sender, branch_location):
+ from bzrlib.branch import Branch
+ branch = Branch.open(branch_location)
+ return branch.revno(), branch.repository.get_revision(branch.last_revision()).message
+

 class BranchSpecOptionHandler:
     """Configuration mechanism for a branch specifications.

=== modified file 'pqm/script.py'
--- pqm/script.py 2008-09-29 08:57:02 +0000
+++ pqm/script.py 2008-11-24 20:09:58 +0000
@@ -402,6 +402,20 @@
         self.successful.append(line)
         self.output += self._success_output(merge_name, start_time)
         self.get_vcs().commit(sender, dir, self.commitmsg, to_repo_revision, config)
+ # Now that we've committed to the branch, let's include the
+ # commit details in the output
+ try:
+ revno, commit_msg = self.get_vcs().latest_revision_info(sender, to_repo_revision)
+ self.successful[-1] = ("%s\n"
+ "Revision: %s \n"
+ "Commit Message: %s"
+ % (line, revno, commit_msg))
+ self.output += ["\n", "Revision Details:", "\n",
+ "Revision: %s" % revno, "\n",
+ "Commit Message: %s" % commit_msg, "\n"]
+ except PQMTlaFailure:
+ # If the latest_revision_info function isn't implemented
+ pass

     def get_arch_impl(self):
         # TODO: Tim Penhey, 2008-05-28

=== modified file 'pqm/tests/test_pqm.py'
--- pqm/tests/test_pqm.py 2008-09-29 08:58:53 +0000
+++ pqm/tests/test_pqm.py 2008-11-24 20:09:58 +0000
@@ -574,6 +574,12 @@
             return
         self.fail("Merge conflict not raised.")

+ def test_latest_revision_info(self):
+ handler = pqm.Bazaar2Handler()
+ revno, message = handler.latest_revision_info("me", "...

Damn! How did that loose all the whitespace?

Crap. No pre!!!

 vote needs_fixing

On Tue, 25 Nov 2008 09:10:28 Tim Penhey wrote:
> === modified file 'pqm/__init__.py'
> --- pqm/__init__.py 2008-09-29 08:57:02 +0000
> +++ pqm/__init__.py 2008-11-24 20:09:58 +0000
> @@ -233,6 +233,9 @@
> def make_local_dir(self, sender, branch, output_dir):
> raise PQMTlaFailure(sender, 'Unsupported operation')
>
> + def latest_revision_info(self, sender, branch):
> + raise PQMTlaFailure(sender, 'Unsupported operation')

Why pass in the sender here? It doesn't make sense to me.

Also I'd like to see a docstring explaining what the expected
return values are.

>
> class ArchHandler(VCSHandler):
>
> @@ -465,6 +468,11 @@
> branch = Branch.open(branch_spec)
> branch.create_checkout(output_dir, lightweight=True)
>
> + def latest_revision_info(self, sender, branch_location):
> + from bzrlib.branch import Branch
> + branch = Branch.open(branch_location)
> + return branch.revno(), branch.repository.get_revision(branch.last_revision()).message
> +
>
> class BranchSpecOptionHandler:
> """Configuration mechanism for a branch specifications.
>
> === modified file 'pqm/script.py'
> --- pqm/script.py 2008-09-29 08:57:02 +0000
> +++ pqm/script.py 2008-11-24 20:09:58 +0000
> @@ -402,6 +402,20 @@
> self.successful.append(line)
> self.output += self._success_output(merge_name, start_time)
> self.get_vcs().commit(sender, dir, self.commitmsg, to_repo_revision, config)
> + # Now that we've committed to the branch, let's include the
> + # commit details in the output
> + try:
> + revno, commit_msg = self.get_vcs().latest_revision_info(sender, to_repo_revision)
> + self.successful[-1] = ("%s\n"
> + "Revision: %s \n"
> + "Commit Message: %s"
> + % (line, revno, commit_msg))
> + self.output += ["\n", "Revision Details:", "\n",

I think extend is better than += for list appending.
Also I think that "Revision Details" is not really needed
here as the next two lines say "Revision" and "Commit Message".
It is fairly obvious that they are the revision details.

> + "Revision: %s" % revno, "\n",
> + "Commit Message: %s" % commit_msg, "\n"]

Wouldn't it be nicer to calculate the revision details once?

   revision_details = "Revision: %s\nCommit Message: %s" % (revno, commit_msg)

Then use that to update the successful string and the output:

   self.successful[-1] = '%s\n%s' % (line, revision_details)
   self.output.append('\n%s\n' % revision_details)

> + except PQMTlaFailure:
> + # If the latest_revision_info function isn't implemented
> + pass
>
> def get_arch_impl(self):
> # TODO: Tim Penhey, 2008-05-28
>
> === modified file 'pqm/tests/test_pqm.py'
> --- pqm/tests/test_pqm.py 2008-09-29 08:58:53 +0000
> +++ pqm/tests/test_pqm.py 2008-11-24 20:09:58 +0000
> @@ -574,6 +574,12 @@
> return
> self.fail("Merge conflict not raised.")
>
> + def test_latest_revision_info(self):

A docstring or comm...

On Wed, 2008-11-26 at 02:39 +0000, Tim Penhey wrote:
> <snip>

This is mthaddon's code, I just tidied it up a bit. I'm happy to look
at this, but don't know when I'll be able to.

On Wed, 26 Nov 2008 23:06:32 Daniel Watkins wrote:
> On Wed, 2008-11-26 at 02:39 +0000, Tim Penhey wrote:
> > <snip>
>
> This is mthaddon's code, I just tidied it up a bit. I'm happy to
> look at this, but don't know when I'll be able to.

OK, I'll talk to mthaddon about this.

This will do wrong things with multiline commit messages - double carriage returns, for instance.

The defect with multiline commit messages, and the unnecessary work are
still there.

However, until merge-directives land the former isn't a concern, and the
latter is performance-not-functional.

-Rob
--