Merge lp:~ian-clatworthy/bzr-builder/minor-code-cleanups into lp:~james-w/bzr-builder/trunk-old

[Ian Clatworthy]

[Ian Clatworthy (ian-clatworthy)]

Some fly-by code clean-ups in __init__.py. Nothing controversial hopefully.

[Robert Collins (lifeless)]

Looks good to me (though it will conflict with ~lifeless/bzr-builder/blocking).

review: Approve

[James Westby (james-w)]

Hi,

Thanks for working on this.

8 +# The default distribution used by get_changelog_entry()
9 +DEFAULT_UBUNTU_DISTRIBUTION = "jaunty"

Doing this makes it clear that this should be "lucid" now.

38 +def run_command(command, msg, error_msg):

69 + _run_command(command,

86 + _run_command(command,

103 + _run_command(command,

These should match shouldn't they?

53 + raise errors.BzrCommandError("%s: %s" % (output, error_msg))

Aren't output and error_msg the wrong way round?

Thanks,

James

review: Needs Fixing

[lp:~ian-clatworthy/bzr-builder/minor-code-cleanups]

56. By Ian Clatworthy on 2009-11-25

Corrections from James' review feedback

[Ian Clatworthy (ian-clatworthy)]

> 8 +# The default distribution used by get_changelog_entry()
> 9 +DEFAULT_UBUNTU_DISTRIBUTION = "jaunty"
>
> Doing this makes it clear that this should be "lucid" now.

Fixed.

> 38 +def run_command(command, msg, error_msg):
>
> 69 + _run_command(command,
>
> 86 + _run_command(command,
>
> 103 + _run_command(command,
>
> These should match shouldn't they?

Fixed.

> 53 + raise errors.BzrCommandError("%s: %s" % (output, error_msg))
>
> Aren't output and error_msg the wrong way round?

Oops. Fixed now.

=== modified file '__init__.py'
--- __init__.py	2009-11-23 16:42:00 +0000
+++ __init__.py	2009-11-25 02:20:24 +0000
@@ -151,6 +151,10 @@
         )
 
 
+# The default distribution used by add_changelog_entry()
+DEFAULT_UBUNTU_DISTRIBUTION = "lucid"
+
+
 def write_manifest_to_path(path, base_branch):
     parent_dir = os.path.dirname(path)
     if parent_dir != '' and not os.path.exists(parent_dir):
@@ -188,8 +192,7 @@
 
 
 def get_maintainer():
-    """
-    Create maintainer string using the same algorithm as in dch
+    """Create maintainer string using the same algorithm as in dch.
     """
     env = os.environ
     regex = re.compile(r"^(.*)\s+<(.*)>$")
@@ -278,7 +281,7 @@
                     "take the package name from, and --package not "
                     "specified.")
         if distribution is None:
-            distribution = "jaunty"
+            distribution = DEFAULT_UBUNTU_DISTRIBUTION
     # Use debian packaging environment variables
     # or default values if they don't exist
     author = "%s <%s>" % get_maintainer()
@@ -295,47 +298,44 @@
         cl_f.close()
 
 
+def _run_command(command, msg, error_msg):
+    """ Run a command in a subprocess.
+
+    :param command: list with command and parameters
+    :param msg: message to display to the user
+    :param error_msg: message to display if something fails.
+    """
+    trace.note(msg)
+    proc = subprocess.Popen(command, cwd=basedir,
+            stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
+            stdin=subprocess.PIPE)
+    proc.stdin.close()
+    retcode = proc.wait()
+    if retcode != 0:
+        output = proc.stdout.read()
+        raise errors.BzrCommandError("%s: %s" % (error_msg, output))
+
+
 def build_source_package(basedir):
-    trace.note("Building the source package")
     command = ["/usr/bin/debuild", "--no-tgz-check", "-i", "-I", "-S",
                     "-uc", "-us"]
-    proc = subprocess.Popen(command, cwd=basedir,
-            stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-            stdin=subprocess.PIPE)
-    proc.stdin.close()
-    retcode = proc.wait()
-    if retcode != 0:
-        output = proc.stdout.read()
-        raise errors.BzrCommandError("Failed to build the source package: "
-                "%s" % output)
+    _run_command(command,
+        "Building the source package",
+        "Failed to build the source package")
 
 
 def sign_source_package(basedir, key_id):
-    trace.note("Signing the source package")
     command = ["/usr/bin/debsign", "-S", "-k%s" % key_id]
-    proc = subprocess.Popen(command, cwd=basedir,
-            stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-            stdin=subprocess.PIPE)
-    proc.stdin.close()
-    retcode = proc.wait()
-    if retcode != 0:
-        output = proc.stdout.read()
-        raise errors.BzrCommandError("Signing the package failed: "
-                "%s" % output)
+    _run_command(command,
+        "Signing the source package",
+        "Signing the package failed")
 
 
 def dput_source_package(basedir, target):
-    trace.note("Uploading the source package")
     command = ["/usr/bin/debrelease", "-S", "--dput", target]
-    proc = subprocess.Popen(command, cwd=basedir,
-            stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-            stdin=subprocess.PIPE)
-    proc.stdin.close()
-    retcode = proc.wait()
-    if retcode != 0:
-        output = proc.stdout.read()
-        raise errors.BzrCommandError("Uploading the package failed: "
-                "%s" % output)
+    _run_command(command,
+        "Uploading the source package",
+        "Uploading the package failed")
 
 
 def get_base_branch(recipe_file, if_changed_from=None):
@@ -414,8 +414,8 @@
                         help="dput the built package to the specified "
                         "dput target."),
                 Option("key-id", type=str, short_name="k",
-                       help="Sign the packages with the specified GnuPG key, "
-                            "must be specified if you use --dput."),
+                       help="Sign the packages with the specified GnuPG key. "
+                            "Must be specified if you use --dput."),
             ]
 
     takes_args = ["recipe_file", "working_directory?"]