Code review comment for lp:~statik/tarmac/testrunner

I can't thank you enough for working on this. Thanks a lot.

 vote approve
 status approved

> === modified file 'tarmac/bin.py'
> --- tarmac/bin.py 2009-03-05 00:29:19 +0000
> +++ tarmac/bin.py 2009-03-05 01:03:23 +0000
> @@ -2,6 +2,7 @@
> '''Code used by Tarmac scripts.'''
> from optparse import OptionParser
> import os
> +import subprocess
> import sys
>
> from bzrlib import branch, bzrdir
> @@ -22,11 +23,14 @@
> def __init__(self):
>
> parser = OptionParser("%prog [options] <projectname>")
> - parser.add_option('--dry-run', action='store_true', dest='dry_run',
> + parser.add_option('--dry-run', action='store_true',
> help='Print out the branches that would be merged and their '
> 'commit messages, but don\'t actually merge the branches.')
> + parser.add_option('--test-command', type='string', default='make test',
> + help='The test command to run after merging a branch.')
> options, args = parser.parse_args()
> self.dry_run = options.dry_run
> + self.test_command = options.test_command
>
> if len(args) != 1:
> # This code is merely a placeholder until I can get proper argument
> @@ -92,7 +96,12 @@
> candidate.source_branch.bzr_identity)
>
> target_tree.merge_from_branch(source_branch)
> - # TODO: Add hook code.
> - target_tree.commit(candidate.all_comments[0].message_body)
> + retcode = subprocess.call(self.test_command, shell=True)
> + if retcode == 0:
> + # TODO: It would be very nice if the commit message included
> + # some reference to the people who voted approve.
> + target_tree.commit(candidate.all_comments[0].message_body)
> + else:
> + target_tree.revert()
>
>
>

Other than this potential conflict, this is really good work, and I'm quite
happy to get it in.

--
Paul Hummer
http://theironlion.net
1024/862FF08F C921 E962 58F8 5547 6723 0E8C 1C4D 8AC5 862F F08F