launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-worker-always-pass-target into lp:launchpad.

Commit message:
Always pass source and target RCS types to the code import worker, since it now always accepts the combined form.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-worker-always-pass-target/+merge/310710
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-worker-always-pass-target into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2016-11-03 11:40:11 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2016-11-12 21:15:04 +0000
@@ -1442,7 +1442,7 @@
             bzr_branch_url="http://example.com/foo";)
         self.assertArgumentsMatch(
             code_import, Equals([
-                str(code_import.branch.id), 'bzr',
+                str(code_import.branch.id), 'bzr:bzr',
                 'http://example.com/foo']))
 
     def test_git_arguments(self):
@@ -1450,7 +1450,7 @@
             git_repo_url="git://git.example.com/project.git")
         self.assertArgumentsMatch(
             code_import, Equals([
-                str(code_import.branch.id), 'git',
+                str(code_import.branch.id), 'git:bzr',
                 'git://git.example.com/project.git']))
 
     def test_git_to_git_arguments(self):
@@ -1472,7 +1472,7 @@
             cvs_root=':pserver:foo@xxxxxxxxxxx/bar', cvs_module='bar')
         self.assertArgumentsMatch(
             code_import, Equals([
-                str(code_import.branch.id), 'cvs',
+                str(code_import.branch.id), 'cvs:bzr',
                 ':pserver:foo@xxxxxxxxxxx/bar', 'bar']))
 
     def test_bzr_svn_arguments(self):
@@ -1480,7 +1480,7 @@
             svn_branch_url='svn://svn.example.com/trunk')
         self.assertArgumentsMatch(
             code_import, Equals([
-                str(code_import.branch.id), 'bzr-svn',
+                str(code_import.branch.id), 'bzr-svn:bzr',
                 'svn://svn.example.com/trunk']))
 
     def test_bzr_stacked(self):
@@ -1491,7 +1491,7 @@
         code_import.branch.stacked_on = devfocus
         self.assertArgumentsMatch(
             code_import, Equals([
-                str(code_import.branch.id), 'bzr',
+                str(code_import.branch.id), 'bzr:bzr',
                 'bzr://bzr.example.com/foo',
                 compose_public_url('http', branch_id_alias(devfocus))]))
 
@@ -1505,5 +1505,5 @@
         code_import.branch.stacked_on = devfocus
         self.assertArgumentsMatch(
             code_import, Equals([
-                str(code_import.branch.id), 'bzr',
+                str(code_import.branch.id), 'bzr:bzr',
                 'bzr://bzr.example.com/foo']))

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2016-11-08 03:27:24 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2016-11-12 21:15:04 +0000
@@ -402,16 +402,8 @@
     def asArguments(self):
         """Return a list of arguments suitable for passing to a child process.
         """
-        result = [str(self.target_id)]
-        if self.target_rcstype == 'bzr':
-            # XXX cjwatson 2016-10-12: Remove this special case once the
-            # worker always accepts the combined form.
-            result.append(self.rcstype)
-        elif self.target_rcstype == 'git':
-            result.append('%s:%s' % (self.rcstype, self.target_rcstype))
-        else:
-            raise AssertionError(
-                "Unknown target_rcstype %r." % self.target_rcstype)
+        result = [
+            str(self.target_id), '%s:%s' % (self.rcstype, self.target_rcstype)]
         if self.rcstype in ['bzr-svn', 'git', 'bzr']:
             result.append(self.url)
             if self.stacked_on_url is not None: