launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/ids-needsbuild into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/ids-needsbuild/+merge/57614

Allow derivation if the parent distroseries has pending builds if and only if the child's distribution is not the same as the parent's. This means they don't share an archive.

I also went mad, and added full stops to every comment that didn't already have them.
-- 
https://code.launchpad.net/~stevenk/launchpad/ids-needsbuild/+merge/57614
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/ids-needsbuild into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/initialise_distroseries.py'
--- lib/lp/soyuz/scripts/initialise_distroseries.py	2011-04-13 05:52:48 +0000
+++ lib/lp/soyuz/scripts/initialise_distroseries.py	2011-04-14 02:10:55 +0000
@@ -81,7 +81,8 @@
                  "derives from {child.parent_series.distribution.name}/"
                  "{child.parent_series.name}.").format(
                     child=self.distroseries))
-        self._checkBuilds()
+        if self.distroseries.parent.id == self.parent.parent.id:
+            self._checkBuilds()
         self._checkQueue()
         self._checkSeries()
 

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2011-04-12 06:44:26 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2011-04-14 02:10:55 +0000
@@ -87,7 +87,8 @@
                     status=PackagePublishingStatus.PUBLISHED)
 
     def test_failure_for_already_released_distroseries(self):
-        # Initialising a distro series that has already been used will error
+        # Initialising a distro series that has already been used will
+        # error.
         child = self.factory.makeDistroSeries()
         self.factory.makeDistroArchSeries(distroseries=child)
         ids = InitialiseDistroSeries(self.parent, child)
@@ -97,20 +98,33 @@
             "distroarchseries(s) initialised for this series.", ids.check)
 
     def test_failure_with_pending_builds(self):
-        # If the parent series has pending builds, we can't initialise
+        # If the parent series has pending builds, and the child is a series
+        # of the same distribution (which means they share an archive), we
+        # can't initialise.
         source = self.factory.makeSourcePackagePublishingHistory(
             distroseries=self.parent,
             pocket=PackagePublishingPocket.RELEASE)
         source.createMissingBuilds()
-        child = self.factory.makeDistroSeries()
+        child = self.factory.makeDistroSeries(
+            distribution=self.parent.parent)
         ids = InitialiseDistroSeries(self.parent, child)
         self.assertRaisesWithContent(
             InitialisationError, "Parent series has pending builds.",
             ids.check)
 
+    def test_success_with_pending_builds(self):
+        # If the parent series has pending builds, and the child's
+        # distribution is different, we can initialise.
+        source = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=self.parent,
+            pocket=PackagePublishingPocket.RELEASE)
+        source.createMissingBuilds()
+        child = self._full_initialise()
+        self.assertDistroSeriesInitialisedCorrectly(child)
+
     def test_failure_with_queue_items(self):
         # If the parent series has items in its queues, such as NEW and
-        # UNAPPROVED, we can't initialise
+        # UNAPPROVED, we can't initialise.
         self.parent.createQueueEntry(
             PackagePublishingPocket.RELEASE,
             'foo.changes', 'bar', self.parent.main_archive)
@@ -121,7 +135,7 @@
             ids.check)
 
     def assertDistroSeriesInitialisedCorrectly(self, child):
-        # Check that 'udev' has been copied correctly
+        # Check that 'udev' has been copied correctly.
         parent_udev_pubs = self.parent.getPublishedSources('udev')
         child_udev_pubs = child.getPublishedSources('udev')
         self.assertEqual(
@@ -132,7 +146,7 @@
             self.parent_das.architecturetag].getReleasedPackages('udev')
         self.assertEqual(
             len(parent_arch_udev_pubs), len(child_arch_udev_pubs))
-        # And the binary package, and linked source package look fine too
+        # And the binary package, and linked source package look fine too.
         udev_bin = child_arch_udev_pubs[0].binarypackagerelease
         self.assertEqual(udev_bin.title, u'udev-0.1-1')
         self.assertEqual(
@@ -149,7 +163,7 @@
             self.parent[self.parent_das.architecturetag],
             self.parent.main_archive)
         self.assertEqual(parent_udev.id, child_udev.id)
-        # We also inherit the permitted source formats from our parent
+        # We also inherit the permitted source formats from our parent.
         self.assertTrue(
             child.isSourcePackageFormatPermitted(
             SourcePackageFormat.FORMAT_1_0))
@@ -166,13 +180,13 @@
         return child
 
     def test_initialise(self):
-        # Test a full initialise with no errors
+        # Test a full initialise with no errors.
         child = self._full_initialise()
         self.assertDistroSeriesInitialisedCorrectly(child)
 
     def test_initialise_only_one_das(self):
         # Test a full initialise with no errors, but only copy i386 to
-        # the child
+        # the child.
         self.factory.makeDistroArchSeries(distroseries=self.parent)
         child = self._full_initialise(
             arches=[self.parent_das.architecturetag])
@@ -184,7 +198,7 @@
             das[0].architecturetag, self.parent_das.architecturetag)
 
     def test_copying_packagesets(self):
-        # If a parent series has packagesets, we should copy them
+        # If a parent series has packagesets, we should copy them.
         uploader = self.factory.makePerson()
         test1 = getUtility(IPackagesetSet).new(
             u'test1', u'test 1 packageset', self.parent.owner,
@@ -199,7 +213,7 @@
         getUtility(IArchivePermissionSet).newPackagesetUploader(
             self.parent.main_archive, uploader, test1)
         child = self._full_initialise()
-        # We can fetch the copied sets from the child
+        # We can fetch the copied sets from the child.
         child_test1 = getUtility(IPackagesetSet).getByName(
             u'test1', distroseries=child)
         child_test2 = getUtility(IPackagesetSet).getByName(
@@ -207,7 +221,7 @@
         child_test3 = getUtility(IPackagesetSet).getByName(
             u'test3', distroseries=child)
         # And we can see they are exact copies, with the related_set for the
-        # copies pointing to the packageset in the parent
+        # copies pointing to the packageset in the parent.
         self.assertEqual(test1.description, child_test1.description)
         self.assertEqual(test2.description, child_test2.description)
         self.assertEqual(test3.description, child_test3.description)
@@ -257,7 +271,7 @@
 
     def test_copy_limit_packagesets(self):
         # If a parent series has packagesets, we can decide which ones we
-        # want to copy
+        # want to copy.
         test1 = getUtility(IPackagesetSet).new(
             u'test1', u'test 1 packageset', self.parent.owner,
             distroseries=self.parent)
@@ -283,7 +297,7 @@
         self.assertEqual(child.binarycount, 2) # Chromium is FTBFS
 
     def test_rebuild_flag(self):
-        # No binaries will get copied if we specify rebuild=True
+        # No binaries will get copied if we specify rebuild=True.
         self.parent.updatePackageCount()
         child = self._full_initialise(rebuild=True)
         child.updatePackageCount()
@@ -296,7 +310,7 @@
 
     def test_limit_packagesets_rebuild_and_one_das(self):
         # We can limit the source packages copied, and only builds
-        # for the copied source will be created
+        # for the copied source will be created.
         test1 = getUtility(IPackagesetSet).new(
             u'test1', u'test 1 packageset', self.parent.owner,
             distroseries=self.parent)
@@ -324,7 +338,7 @@
             das[0].architecturetag, self.parent_das.architecturetag)
 
     def test_do_not_copy_disabled_dases(self):
-        # DASes that are disabled in the parent will not be copied
+        # DASes that are disabled in the parent will not be copied.
         ppc_das = self.factory.makeDistroArchSeries(
             distroseries=self.parent)
         ppc_das.enabled = False
@@ -336,7 +350,7 @@
             das[0].architecturetag, self.parent_das.architecturetag)
 
     def test_script(self):
-        # Do an end-to-end test using the command-line tool
+        # Do an end-to-end test using the command-line tool.
         uploader = self.factory.makePerson()
         test1 = getUtility(IPackagesetSet).new(
             u'test1', u'test 1 packageset', self.parent.owner,