launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/packageset-score into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #990219 in Launchpad itself: "Reprioritize package build scores based on packageset"
  https://bugs.launchpad.net/launchpad/+bug/990219
  Bug #1000570 in Launchpad itself: ""Packageset.score" is badly named"
  https://bugs.launchpad.net/launchpad/+bug/1000570

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/packageset-score/+merge/106220

== Summary ==

Following up on the work in https://code.launchpad.net/~cjwatson/launchpad/packageset-score/+merge/105915, Julian reported in bug 1000570 that the name of Packageset.score is underinformative and inconsistent.

== Proposed fix ==

Rename the field in code to relative_build_score to match Archive.  Renaming the database column is harder at this point, but not especially urgent.

== LOC Rationale ==

+2, but this is a minor tweak to a branch that landed with -100, so I claim credit.

== Tests ==

bin/test -vvct TestBuildPackageJobScore

== Demo and Q/A ==

I don't think any is needed here.
-- 
https://code.launchpad.net/~cjwatson/launchpad/packageset-score/+merge/106220
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/packageset-score into lp:launchpad.

=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
--- lib/lp/soyuz/interfaces/packageset.py	2012-05-16 14:04:41 +0000
+++ lib/lp/soyuz/interfaces/packageset.py	2012-05-17 16:30:25 +0000
@@ -352,7 +352,7 @@
     """A writeable interface for restricted attributes of package sets."""
     export_as_webservice_entry(publish_web_link=False)
 
-    score = exported(Int(
+    relative_build_score = exported(Int(
         title=_("Build score"), required=True, readonly=False,
         description=_("Build score bonus for packages in this package set.")))
 

=== modified file 'lib/lp/soyuz/model/buildpackagejob.py'
--- lib/lp/soyuz/model/buildpackagejob.py	2012-05-15 17:09:59 +0000
+++ lib/lp/soyuz/model/buildpackagejob.py	2012-05-17 16:30:25 +0000
@@ -111,7 +111,7 @@
             self.build.source_package_release.name,
             distroseries=self.build.distro_series)
         if not package_sets.is_empty():
-            score += package_sets.max(Packageset.score)
+            score += package_sets.max(Packageset.relative_build_score)
 
         # Calculates the build queue time component of the score.
         right_now = datetime.now(pytz.timezone('UTC'))

=== modified file 'lib/lp/soyuz/model/packageset.py'
--- lib/lp/soyuz/model/packageset.py	2012-05-16 13:55:07 +0000
+++ lib/lp/soyuz/model/packageset.py	2012-05-17 16:30:25 +0000
@@ -70,7 +70,7 @@
     packagesetgroup_id = Int(name='packagesetgroup', allow_none=False)
     packagesetgroup = Reference(packagesetgroup_id, 'PackagesetGroup.id')
 
-    score = Int(allow_none=False)
+    relative_build_score = Int(name="score", allow_none=False)
 
     def add(self, data):
         """See `IPackageset`."""

=== modified file 'lib/lp/soyuz/tests/test_buildpackagejob.py'
--- lib/lp/soyuz/tests/test_buildpackagejob.py	2012-05-16 17:26:02 +0000
+++ lib/lp/soyuz/tests/test_buildpackagejob.py	2012-05-17 16:30:25 +0000
@@ -377,18 +377,18 @@
             distroseries=job.build.distro_series)
         removeSecurityProxy(packageset).add(
             [job.build.source_package_release.sourcepackagename])
-        removeSecurityProxy(packageset).score = 100
+        removeSecurityProxy(packageset).relative_build_score = 100
         self.assertCorrectScore(job, "RELEASE", "main", "low", 100)
 
     def test_score_packageset_readable(self):
         # A packageset's build score is readable by anyone.
         packageset = self.factory.makePackageset()
-        removeSecurityProxy(packageset).score = 100
+        removeSecurityProxy(packageset).relative_build_score = 100
         webservice = webservice_for_person(
             self.factory.makePerson(), permission=OAuthPermission.WRITE_PUBLIC)
         entry = webservice.get(
             api_url(packageset), api_version="devel").jsonBody()
-        self.assertEqual(100, entry["score"])
+        self.assertEqual(100, entry["relative_build_score"])
 
     def test_score_packageset_forbids_non_buildd_admin(self):
         # Being the owner of a packageset is not enough to allow changing
@@ -400,11 +400,12 @@
         entry = webservice.get(
             api_url(packageset), api_version="devel").jsonBody()
         response = webservice.patch(
-            entry["self_link"], "application/json", dumps(dict(score=100)))
+            entry["self_link"], "application/json",
+            dumps(dict(relative_build_score=100)))
         self.assertEqual(401, response.status)
         new_entry = webservice.get(
             api_url(packageset), api_version="devel").jsonBody()
-        self.assertEqual(0, new_entry["score"])
+        self.assertEqual(0, new_entry["relative_build_score"])
 
     def test_score_packageset_allows_buildd_admin(self):
         buildd_admins = getUtility(IPersonSet).getByName(
@@ -416,6 +417,7 @@
         entry = webservice.get(
             api_url(packageset), api_version="devel").jsonBody()
         response = webservice.patch(
-            entry["self_link"], "application/json", dumps(dict(score=100)))
+            entry["self_link"], "application/json",
+            dumps(dict(relative_build_score=100)))
         self.assertEqual(209, response.status)
-        self.assertEqual(100, response.jsonBody()["score"])
+        self.assertEqual(100, response.jsonBody()["relative_build_score"])