launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/recipe-name-policy into lp:launchpad with lp:~cjwatson/launchpad/git-recipe-browser-existing as a prerequisite.

Commit message:
Add allow_recipe_name_from_target to IBranchTarget and IGitNamespacePolicy, so that SourcePackageRecipeAddView doesn't have to do isinstance checks.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1453022 in Launchpad itself: "Please support daily builds via git"
  https://bugs.launchpad.net/launchpad/+bug/1453022

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/recipe-name-policy/+merge/282303

Add allow_recipe_name_from_target to IBranchTarget and IGitNamespacePolicy, so that SourcePackageRecipeAddView doesn't have to do isinstance checks.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/recipe-name-policy into lp:launchpad.

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2016-01-12 14:17:30 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2016-01-12 14:17:30 +0000
@@ -57,7 +57,6 @@
     SimpleTerm,
     SimpleVocabulary,
     )
-from zope.security.proxy import isinstance as zope_isinstance
 
 from lp import _
 from lp.app.browser.launchpadform import (
@@ -99,7 +98,6 @@
     ISourcePackageRecipeSource,
     MINIMAL_RECIPE_TEXT_BZR,
     )
-from lp.code.model.branchtarget import PersonBranchTarget
 from lp.code.vocabularies.sourcepackagerecipe import BuildableDistroSeries
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.fields import PersonChoice
@@ -746,10 +744,10 @@
         """A generator of recipe names."""
         # +junk-daily doesn't make a very good recipe name, so use the
         # branch name in that case.
-        if zope_isinstance(self.context.target, PersonBranchTarget):
+        if self.context.target.allow_recipe_name_from_target:
+            branch_target_name = self.context.target.name.split('/')[-1]
+        else:
             branch_target_name = self.context.name
-        else:
-            branch_target_name = self.context.target.name.split('/')[-1]
         yield "%s-daily" % branch_target_name
         counter = itertools.count(1)
         while True:

=== modified file 'lib/lp/code/interfaces/branchtarget.py'
--- lib/lp/code/interfaces/branchtarget.py	2015-09-28 23:50:43 +0000
+++ lib/lp/code/interfaces/branchtarget.py	2016-01-12 14:17:30 +0000
@@ -98,6 +98,10 @@
     supports_code_imports = Attribute(
         "Does this target support code imports at all?")
 
+    allow_recipe_name_from_target = Attribute(
+        "Can recipe names reasonably be generated from the target name "
+        "rather than the branch name?")
+
     def areBranchesMergeable(other_target):
         """Are branches from other_target mergeable into this target?"""
 

=== modified file 'lib/lp/code/interfaces/gitnamespace.py'
--- lib/lp/code/interfaces/gitnamespace.py	2015-04-28 23:25:08 +0000
+++ lib/lp/code/interfaces/gitnamespace.py	2016-01-12 14:17:30 +0000
@@ -96,6 +96,10 @@
     supports_merge_proposals = Attribute(
         "Does this namespace support merge proposals at all?")
 
+    allow_recipe_name_from_target = Attribute(
+        "Can recipe names reasonably be generated from the target name "
+        "rather than the branch name?")
+
     def getAllowedInformationTypes(who):
         """Get the information types that a repository in this namespace can
         have.

=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py	2015-09-28 23:50:43 +0000
+++ lib/lp/code/model/branchtarget.py	2016-01-12 14:17:30 +0000
@@ -122,6 +122,11 @@
         """See `IBranchTarget`."""
         return True
 
+    @property
+    def allow_recipe_name_from_target(self):
+        """See `IBranchTarget`."""
+        return True
+
     def areBranchesMergeable(self, other_target):
         """See `IBranchTarget`."""
         # Branches are mergable into a PackageTarget if the source package
@@ -238,6 +243,11 @@
         """See `IBranchTarget`."""
         return False
 
+    @property
+    def allow_recipe_name_from_target(self):
+        """See `IBranchTarget`."""
+        return False
+
     def areBranchesMergeable(self, other_target):
         """See `IBranchTarget`."""
         return False
@@ -323,6 +333,11 @@
         """See `IBranchTarget`."""
         return True
 
+    @property
+    def allow_recipe_name_from_target(self):
+        """See `IBranchTarget`."""
+        return True
+
     def areBranchesMergeable(self, other_target):
         """See `IBranchTarget`."""
         # Branches are mergable into a PackageTarget if the source package

=== modified file 'lib/lp/code/model/gitnamespace.py'
--- lib/lp/code/model/gitnamespace.py	2015-10-09 15:49:14 +0000
+++ lib/lp/code/model/gitnamespace.py	2016-01-12 14:17:30 +0000
@@ -237,6 +237,7 @@
     has_defaults = False
     allow_push_to_set_default = False
     supports_merge_proposals = False
+    allow_recipe_name_from_target = False
 
     def __init__(self, person):
         self.owner = person
@@ -314,6 +315,7 @@
     has_defaults = True
     allow_push_to_set_default = True
     supports_merge_proposals = True
+    allow_recipe_name_from_target = True
 
     def __init__(self, person, project):
         self.owner = person
@@ -398,6 +400,7 @@
     has_defaults = True
     allow_push_to_set_default = False
     supports_merge_proposals = True
+    allow_recipe_name_from_target = True
 
     def __init__(self, person, distro_source_package):
         self.owner = person

=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py	2015-09-28 23:50:43 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py	2016-01-12 14:17:30 +0000
@@ -215,6 +215,9 @@
         self.assertEqual(owner, code_import.branch.owner)
         self.assertEqual(self.target, code_import.branch.target)
 
+    def test_allow_recipe_name_from_target(self):
+        self.assertTrue(self.target.allow_recipe_name_from_target)
+
     def test_related_branches(self):
         (branch, related_series_branch_info,
             related_package_branches) = (
@@ -335,6 +338,9 @@
                 self.factory.getUniqueString("name-"),
                 RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
 
+    def test_does_not_allow_recipe_name_from_target(self):
+        self.assertFalse(self.target.allow_recipe_name_from_target)
+
 
 class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
 
@@ -470,6 +476,9 @@
         self.assertEqual(owner, code_import.branch.owner)
         self.assertEqual(self.target, code_import.branch.target)
 
+    def test_allow_recipe_name_from_target(self):
+        self.assertTrue(self.target.allow_recipe_name_from_target)
+
     def test_related_branches(self):
         (branch, related_series_branch_info,
             related_package_branches) = (