Launchpad itself

Merge lp:~james-w/launchpad/new-code-import-method-on-branchtarget into lp:launchpad

  1. new-code-import-method-on-branchtarget
  2. Merge into devel
Proposed by James Westby
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/new-code-import-method-on-branchtarget
Merge into: lp:launchpad
Prerequisite: lp:~james-w/launchpad/remove-code-import-list
Diff against target: 234 lines (+108/-2)
5 files modified
lib/lp/code/interfaces/branchtarget.py (+18/-0)
lib/lp/code/model/branchtarget.py (+27/-0)
lib/lp/code/model/codeimport.py (+2/-0)
lib/lp/code/model/tests/test_branchtarget.py (+49/-1)
lib/lp/code/model/tests/test_codeimport.py (+12/-1)
To merge this branch: bzr merge lp:~james-w/launchpad/new-code-import-method-on-branchtarget
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+22190@code.launchpad.net

Commit message

New method IBranchTarget.newCodeImport() to create code imports for a given target.

Description of the change

Hi,

This adds a newCodeImport() method on IBranchTarget that is quite
a simple wrapper around ICodeImportSet.new().

The reason for this is basically to export it later from that
position.

It also adds a new property to IBranchTarget, supports_code_imports,
which indicates, surprisingly, whether the target supports code imports.
This is in the spirit of supports_merge_proposals. Currently it is
only enabled for products, so there is no change from before.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :
Download full text (4.9 KiB)

Two minor whitespace tweaks please, or rather the same one in two
places. Other than that, awesome!

> === modified file 'lib/lp/code/interfaces/branchtarget.py'
> --- lib/lp/code/interfaces/branchtarget.py 2010-01-05 08:18:23 +0000
> +++ lib/lp/code/interfaces/branchtarget.py 2010-03-26 01:45:34 +0000
> @@ -88,6 +88,9 @@
> supports_merge_proposals = Attribute(
> "Does this target support merge proposals at all?")
>
> + supports_code_imports = Attribute(
> + "Does this target support code imports at all?")
> +
> def areBranchesMergeable(other_target):
> """Are branches from other_target mergeable into this target."""
>
> @@ -113,3 +116,18 @@
>
> def getBugTask(bug):
> """Get the BugTask for a given bug related to the branch target."""
> +
> + def newCodeImport(registrant, branch_name, rcs_type,
> + url=None, cvs_root=None, cvs_module=None):

This should be formatted like so:

    def newCodeImport(registrant, branch_name, rcs_type, url=None,
                      cvs_root=None, cvs_module=None):

(https://dev.launchpad.net/PythonStyleGuide#Wrapping%20long%20arguments%20in%20function%20definitions)

> + """Create a new code import for this target.
> +
> + :param registrant: the `IPerson` who should be recorded as creating
> + the import and will own the resulting branch.
> + :param branch_name: the name the resulting branch should have.
> + :param rcs_type: the type of the foreign VCS.
> + :param url: the url to import from if the import isn't CVS.
> + :param cvs_root: if the import is from CVS the CVSROOT to import from.
> + :param cvs_module: if the import is from CVS the module to import.
> + :returns: an `ICodeImport`.
> + :raises AssertionError: if supports_code_imports is False.
> + """

> === modified file 'lib/lp/code/model/branchtarget.py'
> --- lib/lp/code/model/branchtarget.py 2010-01-05 08:18:23 +0000
> +++ lib/lp/code/model/branchtarget.py 2010-03-26 01:45:34 +0000
> @@ -19,6 +19,7 @@
> from lp.code.interfaces.branchcollection import IAllBranches
> from lp.code.interfaces.branchtarget import (
> check_default_stacked_on, IBranchTarget)
> +from lp.code.interfaces.codeimport import ICodeImportSet
> from lp.registry.interfaces.pocket import PackagePublishingPocket
> from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
>
> @@ -36,6 +37,12 @@
> def __ne__(self, other):
> return self.context != other.context
>
> + def newCodeImport(self, registrant, branch_name, rcs_type, url=None,
> + cvs_root=None, cvs_module=None):

Similarly.

> + return getUtility(ICodeImportSet).new(
> + registrant, self, branch_name, rcs_type, url=url,
> + cvs_root=cvs_root, cvs_module=cvs_module)
> +
>
> class PackageBranchTarget(_BaseBranchTarget):
> implements(IBranchTarget)
> @@ -95,6 +102,11 @@
> """See `IBranchTarget`."""
> return True
>
> + @property
> + def supports_code_imports(self):
> + """See `IBranchTarget`."""
> + return False
> +
> def areBranchesMergeable(self, other_target)...

Read more...

Revision history for this message
Michael Hudson-Doyle (mwhudson) :
review: Approve
Revision history for this message
James Westby (james-w) wrote :

Whitespace issues fixed.

Thanks,

James

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/interfaces/branchtarget.py'
--- lib/lp/code/interfaces/branchtarget.py 2010-01-05 08:18:23 +0000
+++ lib/lp/code/interfaces/branchtarget.py 2010-04-02 02:44:29 +0000
@@ -88,6 +88,9 @@
88 supports_merge_proposals = Attribute(88 supports_merge_proposals = Attribute(
89 "Does this target support merge proposals at all?")89 "Does this target support merge proposals at all?")
9090
91 supports_code_imports = Attribute(
92 "Does this target support code imports at all?")
93
91 def areBranchesMergeable(other_target):94 def areBranchesMergeable(other_target):
92 """Are branches from other_target mergeable into this target."""95 """Are branches from other_target mergeable into this target."""
9396
@@ -113,3 +116,18 @@
113116
114 def getBugTask(bug):117 def getBugTask(bug):
115 """Get the BugTask for a given bug related to the branch target."""118 """Get the BugTask for a given bug related to the branch target."""
119
120 def newCodeImport(registrant, branch_name, rcs_type, url=None,
121 cvs_root=None, cvs_module=None):
122 """Create a new code import for this target.
123
124 :param registrant: the `IPerson` who should be recorded as creating
125 the import and will own the resulting branch.
126 :param branch_name: the name the resulting branch should have.
127 :param rcs_type: the type of the foreign VCS.
128 :param url: the url to import from if the import isn't CVS.
129 :param cvs_root: if the import is from CVS the CVSROOT to import from.
130 :param cvs_module: if the import is from CVS the module to import.
131 :returns: an `ICodeImport`.
132 :raises AssertionError: if supports_code_imports is False.
133 """
116134
=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py 2010-01-05 08:18:23 +0000
+++ lib/lp/code/model/branchtarget.py 2010-04-02 02:44:29 +0000
@@ -19,6 +19,7 @@
19from lp.code.interfaces.branchcollection import IAllBranches19from lp.code.interfaces.branchcollection import IAllBranches
20from lp.code.interfaces.branchtarget import (20from lp.code.interfaces.branchtarget import (
21 check_default_stacked_on, IBranchTarget)21 check_default_stacked_on, IBranchTarget)
22from lp.code.interfaces.codeimport import ICodeImportSet
22from lp.registry.interfaces.pocket import PackagePublishingPocket23from lp.registry.interfaces.pocket import PackagePublishingPocket
23from canonical.launchpad.webapp.interfaces import ICanonicalUrlData24from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
2425
@@ -36,6 +37,12 @@
36 def __ne__(self, other):37 def __ne__(self, other):
37 return self.context != other.context38 return self.context != other.context
3839
40 def newCodeImport(self, registrant, branch_name, rcs_type, url=None,
41 cvs_root=None, cvs_module=None):
42 return getUtility(ICodeImportSet).new(
43 registrant, self, branch_name, rcs_type, url=url,
44 cvs_root=cvs_root, cvs_module=cvs_module)
45
3946
40class PackageBranchTarget(_BaseBranchTarget):47class PackageBranchTarget(_BaseBranchTarget):
41 implements(IBranchTarget)48 implements(IBranchTarget)
@@ -95,6 +102,11 @@
95 """See `IBranchTarget`."""102 """See `IBranchTarget`."""
96 return True103 return True
97104
105 @property
106 def supports_code_imports(self):
107 """See `IBranchTarget`."""
108 return False
109
98 def areBranchesMergeable(self, other_target):110 def areBranchesMergeable(self, other_target):
99 """See `IBranchTarget`."""111 """See `IBranchTarget`."""
100 # Branches are mergable into a PackageTarget if the source package112 # Branches are mergable into a PackageTarget if the source package
@@ -182,6 +194,11 @@
182 """See `IBranchTarget`."""194 """See `IBranchTarget`."""
183 return False195 return False
184196
197 @property
198 def supports_code_imports(self):
199 """See `IBranchTarget`."""
200 return False
201
185 def areBranchesMergeable(self, other_target):202 def areBranchesMergeable(self, other_target):
186 """See `IBranchTarget`."""203 """See `IBranchTarget`."""
187 return False204 return False
@@ -258,6 +275,11 @@
258 """See `IBranchTarget`."""275 """See `IBranchTarget`."""
259 return True276 return True
260277
278 @property
279 def supports_code_imports(self):
280 """See `IBranchTarget`."""
281 return True
282
261 def areBranchesMergeable(self, other_target):283 def areBranchesMergeable(self, other_target):
262 """See `IBranchTarget`."""284 """See `IBranchTarget`."""
263 # Branches are mergable into a PackageTarget if the source package285 # Branches are mergable into a PackageTarget if the source package
@@ -311,6 +333,11 @@
311 """See `IBranchTarget`."""333 """See `IBranchTarget`."""
312 return self.productseries334 return self.productseries
313335
336 @property
337 def supports_code_imports(self):
338 """See `IBranchTarget`."""
339 return False
340
314341
315def get_canonical_url_data_for_target(branch_target):342def get_canonical_url_data_for_target(branch_target):
316 """Return the `ICanonicalUrlData` for an `IBranchTarget`."""343 """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
317344
=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py 2010-04-02 02:44:28 +0000
+++ lib/lp/code/model/codeimport.py 2010-04-02 02:44:29 +0000
@@ -225,6 +225,8 @@
225 review_status = CodeImportReviewStatus.REVIEWED225 review_status = CodeImportReviewStatus.REVIEWED
226 else:226 else:
227 review_status = CodeImportReviewStatus.NEW227 review_status = CodeImportReviewStatus.NEW
228 if not target.supports_code_imports:
229 raise AssertionError("%r doesn't support code imports" % target)
228 # Create the branch for the CodeImport.230 # Create the branch for the CodeImport.
229 namespace = target.getNamespace(registrant)231 namespace = target.getNamespace(registrant)
230 import_branch = namespace.createBranch(232 import_branch = namespace.createBranch(
231233
=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py 2010-03-18 15:39:58 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py 2010-04-02 02:44:29 +0000
@@ -14,8 +14,9 @@
14 check_default_stacked_on,14 check_default_stacked_on,
15 PackageBranchTarget, PersonBranchTarget, ProductBranchTarget,15 PackageBranchTarget, PersonBranchTarget, ProductBranchTarget,
16 ProductSeriesBranchTarget)16 ProductSeriesBranchTarget)
17from lp.code.enums import BranchType17from lp.code.enums import BranchType, RevisionControlSystems
18from lp.code.interfaces.branchtarget import IBranchTarget18from lp.code.interfaces.branchtarget import IBranchTarget
19from lp.code.interfaces.codeimport import ICodeImport
19from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities20from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
20from lp.registry.interfaces.pocket import PackagePublishingPocket21from lp.registry.interfaces.pocket import PackagePublishingPocket
21from canonical.launchpad.webapp import canonical_url22from canonical.launchpad.webapp import canonical_url
@@ -176,6 +177,16 @@
176 ubuntu_branches.teamowner)177 ubuntu_branches.teamowner)
177 self.assertEqual(branch, self.target.default_merge_target)178 self.assertEqual(branch, self.target.default_merge_target)
178179
180 def test_doesnt_support_code_imports(self):
181 self.assertFalse(self.target.supports_code_imports)
182
183 def test_creating_code_import_fails(self):
184 self.assertRaises(
185 AssertionError, self.target.newCodeImport,
186 self.factory.makePerson(),
187 self.factory.getUniqueString("name-"),
188 RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
189
179190
180class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):191class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
181192
@@ -258,6 +269,16 @@
258 self.target._retargetBranch(removeSecurityProxy(branch))269 self.target._retargetBranch(removeSecurityProxy(branch))
259 self.assertEqual(self.target, branch.target)270 self.assertEqual(self.target, branch.target)
260271
272 def test_doesnt_support_code_imports(self):
273 self.assertFalse(self.target.supports_code_imports)
274
275 def test_creating_code_import_fails(self):
276 self.assertRaises(
277 AssertionError, self.target.newCodeImport,
278 self.factory.makePerson(),
279 self.factory.getUniqueString("name-"),
280 RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
281
261282
262class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):283class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
263284
@@ -364,6 +385,23 @@
364 setattr, self.original.development_focus, 'branch', branch)385 setattr, self.original.development_focus, 'branch', branch)
365 self.assertEqual(branch, self.target.default_merge_target)386 self.assertEqual(branch, self.target.default_merge_target)
366387
388 def test_supports_code_imports(self):
389 self.assertTrue(self.target.supports_code_imports)
390
391 def test_creating_code_import_succeeds(self):
392 target_url = self.factory.getUniqueURL()
393 branch_name = self.factory.getUniqueString("name-")
394 owner = self.factory.makePerson()
395 code_import = self.target.newCodeImport(
396 owner, branch_name, RevisionControlSystems.GIT, url=target_url)
397 code_import = removeSecurityProxy(code_import)
398 self.assertProvides(code_import, ICodeImport)
399 self.assertEqual(target_url, code_import.url)
400 self.assertEqual(branch_name, code_import.branch.name)
401 self.assertEqual(owner, code_import.registrant)
402 self.assertEqual(owner, code_import.branch.owner)
403 self.assertEqual(self.target, code_import.branch.target)
404
367405
368class TestProductSeriesBranchTarget(TestCaseWithFactory):406class TestProductSeriesBranchTarget(TestCaseWithFactory):
369407
@@ -378,6 +416,16 @@
378 target = IBranchTarget(self.original)416 target = IBranchTarget(self.original)
379 self.assertIsInstance(target, ProductSeriesBranchTarget)417 self.assertIsInstance(target, ProductSeriesBranchTarget)
380418
419 def test_doesnt_support_code_imports(self):
420 self.assertFalse(self.target.supports_code_imports)
421
422 def test_creating_code_import_fails(self):
423 self.assertRaises(
424 AssertionError, self.target.newCodeImport,
425 self.factory.makePerson(),
426 self.factory.getUniqueString("name-"),
427 RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
428
381429
382class TestCheckDefaultStackedOnBranch(TestCaseWithFactory):430class TestCheckDefaultStackedOnBranch(TestCaseWithFactory):
383 """Only certain branches are allowed to be default stacked-on branches."""431 """Only certain branches are allowed to be default stacked-on branches."""
384432
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:44:28 +0000
+++ lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:44:29 +0000
@@ -120,9 +120,20 @@
120 self.assertEqual(120 self.assertEqual(
121 CodeImportReviewStatus.REVIEWED,121 CodeImportReviewStatus.REVIEWED,
122 code_import.review_status)122 code_import.review_status)
123 # No job is created for the import.123 # A job is created for the import.
124 self.assertIsNot(None, code_import.import_job)124 self.assertIsNot(None, code_import.import_job)
125125
126 def test_junk_code_import_rejected(self):
127 """You are not allowed to create code imports targetting +junk."""
128 registrant = self.factory.makePerson()
129 self.assertRaises(AssertionError, CodeImportSet().new,
130 registrant=registrant,
131 target=IBranchTarget(registrant),
132 branch_name='imported',
133 rcs_type=RevisionControlSystems.HG,
134 url=self.factory.getUniqueURL(),
135 review_status=None)
136
126137
127class TestCodeImportDeletion(TestCaseWithFactory):138class TestCodeImportDeletion(TestCaseWithFactory):
128 """Test the deletion of CodeImports."""139 """Test the deletion of CodeImports."""