launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/pcj-repr into lp:launchpad.

Commit message:
Improve representation of PPAs in PlainPackageCopyJob.__repr__.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #812818 in Launchpad itself: "PackageCopyJob __repr__ doesn't show enough info"
  https://bugs.launchpad.net/launchpad/+bug/812818

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/pcj-repr/+merge/226009

Improve representation of PPAs in PlainPackageCopyJob.__repr__.

PPAs are now shown as "~owner/distribution/name".
-- 
https://code.launchpad.net/~cjwatson/launchpad/pcj-repr/+merge/226009
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/pcj-repr into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py	2014-05-15 08:49:44 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py	2014-07-08 17:13:46 +0000
@@ -735,23 +735,25 @@
 
     def __repr__(self):
         """Returns an informative representation of the job."""
+        def archive_name(archive):
+            if archive.is_ppa:
+                return "~%s/%s/%s" % (
+                    archive.owner.name, archive.distribution.name,
+                    archive.name)
+            else:
+                return "%s/%s" % (archive.distribution.name, archive.name)
+
         parts = ["%s to copy" % self.__class__.__name__]
         if self.package_name is None:
             parts.append(" no package (!)")
         else:
             parts.append(" package %s" % self.package_name)
-        parts.append(
-            " from %s/%s" % (
-                self.source_archive.distribution.name,
-                self.source_archive.name))
+        parts.append(" from %s" % archive_name(self.source_archive))
         if self.source_pocket is not None:
             parts.append(", %s pocket," % self.source_pocket.name)
         if self.source_distroseries is not None:
             parts.append(" in %s" % self.source_distroseries)
-        parts.append(
-            " to %s/%s" % (
-                self.target_archive.distribution.name,
-                self.target_archive.name))
+        parts.append(" to %s" % archive_name(self.target_archive))
         parts.append(", %s pocket," % self.target_pocket.name)
         if self.target_distroseries is not None:
             parts.append(" in %s" % self.target_distroseries)

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2014-07-08 06:34:37 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2014-07-08 17:13:46 +0000
@@ -593,8 +593,11 @@
 
     def test___repr__(self):
         distroseries = self.factory.makeDistroSeries()
-        archive1 = self.factory.makeArchive(distroseries.distribution)
-        archive2 = self.factory.makeArchive(distroseries.distribution)
+        owner = self.factory.makePerson()
+        archive1 = self.factory.makeArchive(
+            distribution=distroseries.distribution, owner=owner)
+        archive2 = self.factory.makeArchive(
+            distribution=distroseries.distribution, owner=owner)
         requester = self.factory.makePerson()
         source = getUtility(IPlainPackageCopyJobSource)
         job = source.create(
@@ -605,12 +608,13 @@
             requester=requester)
         self.assertEqual(
             ("<PlainPackageCopyJob to copy package foo from "
-             "{distroseries.distribution.name}/{archive1.name} to "
-             "{distroseries.distribution.name}/{archive2.name}, "
-             "RELEASE pocket, in {distroseries.distribution.name} "
-             "{distroseries.name}, including binaries>").format(
-                distroseries=distroseries, archive1=archive1,
-                archive2=archive2),
+             "~{owner.name}/{distribution.name}/{archive1.name} to "
+             "~{owner.name}/{distribution.name}/{archive2.name}, "
+             "RELEASE pocket, in {distribution.name} {distroseries.name}, "
+             "including binaries>").format(
+                owner=owner, distribution=distroseries.distribution,
+                archive1=archive1, archive2=archive2,
+                distroseries=distroseries),
             repr(job))
 
     def test_getPendingJobsPerPackage_finds_jobs(self):