yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/python-shell-toolbox/add-extra-repositories into lp:python-shell-toolbox.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/python-shell-toolbox/add-extra-repositories/+merge/95567

== Changes ==

- Fixed a typo in `install_extra_repositories`.
- `install_extra_repositories` now works correctly cross-distributions.
- `run` helper improvements:
   - None is ignored if passed as positional argument.
   - The function accepts popen kwargs.

-- 
https://code.launchpad.net/~frankban/python-shell-toolbox/add-extra-repositories/+merge/95567
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/python-shell-toolbox/add-extra-repositories into lp:python-shell-toolbox.

=== modified file 'shelltoolbox/__init__.py'
--- shelltoolbox/__init__.py	2012-02-29 21:41:28 +0000
+++ shelltoolbox/__init__.py	2012-03-02 14:05:21 +0000
@@ -50,14 +50,18 @@
 Env = namedtuple('Env', 'uid gid home')
 
 
-def run(*args):
+def run(*args, **kwargs):
     """Run the command with the given arguments.
 
-    The first argument is the path to the command to run, subsequent arguments
-    are command-line arguments to be passed.
+    The first argument is the path to the command to run.
+    Subsequent arguments are command-line arguments to be passed.
     """
-    process = subprocess.Popen(args, stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE, close_fds=True)
+    args = [i for i in args if i is not None]
+    pipe = subprocess.PIPE
+    process = subprocess.Popen(
+        args, stdout=kwargs.pop('stdout', pipe),
+        stderr=kwargs.pop('stderr', pipe),
+        close_fds=kwargs.pop('close_fds', True), **kwargs)
     stdout, stderr = process.communicate()
     if process.returncode:
         raise subprocess.CalledProcessError(
@@ -98,9 +102,11 @@
 
     :raises: subprocess.CalledProcessError
     """
+    distribution = run('lsb_release', '-cs').strip()
+    assume_yes = None if distribution == 'lucid' else '-y'
     for repo in repositories:
-        run('apt-add-repository', repo)
-    run('apt-get', 'clear')
+        run('apt-add-repository', assume_yes, repo)
+    run('apt-get', 'clean')
     run('apt-get', 'update')
 
 

=== modified file 'tests.py'
--- tests.py	2012-02-28 22:38:27 +0000
+++ tests.py	2012-03-02 14:05:21 +0000
@@ -38,9 +38,13 @@
             run('ls', '--not a valid switch')
         exception = info.exception
         self.assertEqual(2, exception.returncode)
-        self.assertEqual("('ls', '--not a valid switch')", exception.cmd)
+        self.assertEqual("['ls', '--not a valid switch']", exception.cmd)
         self.assertIn('unrecognized option', exception.output)
 
+    def testNoneArguments(self):
+        # Ensure None is ignored when passed as positional argument.
+        self.assertIn('Usage:', run('/bin/ls', None, '--help', None))
+
 
 class TestCommand(unittest.TestCase):