launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:oci-allow-default-git-repository into launchpad:master.

Commit message:
Remove restriction on default Git repository for OCI projects

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/401605

We originally restricted this because we wanted to tie it to official recipe status, but later realized that didn't quite make sense as it's reasonable and useful to have multiple official recipes for a single OCI project.  There doesn't seem any reason to retain this restriction now.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:oci-allow-default-git-repository into launchpad:master.

diff --git a/lib/lp/code/browser/tests/test_gitlisting.py b/lib/lp/code/browser/tests/test_gitlisting.py
index 14239ad..fab5cd4 100644
--- a/lib/lp/code/browser/tests/test_gitlisting.py
+++ b/lib/lp/code/browser/tests/test_gitlisting.py
@@ -428,10 +428,6 @@ class TestOCIProjectGitListingView(
             pillar=distro, ociprojectname="bar")
         self.target_path = "foo/+oci/bar"
 
-    def setDefaultRepository(self, target, repository):
-        getUtility(IGitRepositorySet).setDefaultRepository(
-            target=target, repository=repository, force_oci=True)
-
     def test_bzr_link(self):
         # There's no OCIProject:+branches, nor any ability to create Bazaar
         # branches for OCI projects.
diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index 5374fb7..f486738 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -1161,7 +1161,7 @@ class IGitRepositorySet(Interface):
             title=_("Git repository"), required=False, schema=IGitRepository))
     @export_write_operation()
     @operation_for_version("devel")
-    def setDefaultRepository(target, repository, force_oci=False):
+    def setDefaultRepository(target, repository):
         """Set the default repository for a target.
 
         :param target: An `IHasGitRepositories`.
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 2c66571..78a2c5c 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1908,12 +1908,8 @@ class GitRepositorySet:
                 "Personal repositories cannot be defaults for any target.")
         return IStore(GitRepository).find(GitRepository, *clauses).one()
 
-    def setDefaultRepository(self, target, repository, force_oci=False):
+    def setDefaultRepository(self, target, repository):
         """See `IGitRepositorySet`."""
-        if IOCIProject.providedBy(target) and not force_oci:
-            raise GitTargetError(
-                "Cannot manually set a default Git repository"
-                " for an OCI Project")
         if IPerson.providedBy(target):
             raise GitTargetError(
                 "Cannot set a default Git repository for a person, only "
diff --git a/lib/lp/code/model/tests/test_gitlookup.py b/lib/lp/code/model/tests/test_gitlookup.py
index 69f209b..2ec1e62 100644
--- a/lib/lp/code/model/tests/test_gitlookup.py
+++ b/lib/lp/code/model/tests/test_gitlookup.py
@@ -173,7 +173,7 @@ class TestGetByPath(TestCaseWithFactory):
         repository = self.factory.makeGitRepository(target=oci_project)
         with person_logged_in(repository.target.distribution.owner):
             getUtility(IGitRepositorySet).setDefaultRepository(
-                repository.target, repository, force_oci=True)
+                repository.target, repository)
         self.assertEqual(
             (repository, ""), self.lookup.getByPath(repository.shortened_path))
 
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 5790338..0d0078d 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -634,8 +634,7 @@ class TestGitIdentityMixin(TestCaseWithFactory):
         oci_project = self.factory.makeOCIProject()
         repository = self.factory.makeGitRepository(target=oci_project)
         with admin_logged_in():
-            self.repository_set.setDefaultRepository(
-                oci_project, repository, force_oci=True)
+            self.repository_set.setDefaultRepository(oci_project, repository)
         self.assertGitIdentity(
             repository,
             "%s/+oci/%s" % (oci_project.pillar.name, oci_project.name))
@@ -751,8 +750,7 @@ class TestGitIdentityMixin(TestCaseWithFactory):
         with admin_logged_in():
             self.repository_set.setDefaultRepositoryForOwner(
                 repository.owner, oci_project, repository, repository.owner)
-            self.repository_set.setDefaultRepository(
-                oci_project, repository, force_oci=True)
+            self.repository_set.setDefaultRepository(oci_project, repository)
         eric_oci_project = getUtility(IPersonOCIProjectFactory).create(
             eric, oci_project)
         self.assertEqual(
@@ -3595,29 +3593,6 @@ class TestGitRepositorySet(TestCaseWithFactory):
                 self.repository_set.setDefaultRepositoryForOwner,
                 person, person, repository, user)
 
-    def test_setDefaultRepository_refuses_oci_project(self):
-        # setDefaultRepository refuses if the target is an OCI project.
-        oci_project = self.factory.makeOCIProject()
-        repository = self.factory.makeGitRepository(target=oci_project)
-        with admin_logged_in():
-            self.assertRaises(
-                GitTargetError, self.repository_set.setDefaultRepository,
-                oci_project, repository)
-
-    def test_setDefaultRepository_accepts_oci_project_override(self):
-        # setDefaultRepository refuses if the target is an OCI project.
-        oci_project = self.factory.makeOCIProject()
-        repository = self.factory.makeGitRepository(target=oci_project)
-        with admin_logged_in():
-            self.repository_set.setDefaultRepository(
-                oci_project, repository, force_oci=True)
-        identity_path = "%s/+oci/%s" % (
-                oci_project.distribution.name, oci_project.name)
-        self.assertEqual(
-            identity_path, repository.shortened_path, "shortened path")
-        self.assertEqual(
-            "lp:%s" % identity_path, repository.git_identity, "git identity")
-
     def test_setDefaultRepositoryForOwner_noop(self):
         # If a repository is already the target owner default, setting
         # the default again should no-op.
@@ -3767,12 +3742,6 @@ class TestGitRepositorySetDefaultsPackage(
 class TestGitRepositorySetDefaultsOCIProject(
     TestGitRepositorySetDefaultsMixin, TestCaseWithFactory):
 
-    def setUp(self):
-        super(TestGitRepositorySetDefaultsOCIProject, self).setUp()
-        self.set_method = (lambda target, repository, user:
-            self.repository_set.setDefaultRepository(
-                target, repository, force_oci=True))
-
     def makeTarget(self, template=None):
         kwargs = {}
         if template is not None: