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
1=== modified file 'lib/lp/code/interfaces/branchtarget.py'
2--- lib/lp/code/interfaces/branchtarget.py 2010-01-05 08:18:23 +0000
3+++ lib/lp/code/interfaces/branchtarget.py 2010-04-02 02:44:29 +0000
4@@ -88,6 +88,9 @@
5 supports_merge_proposals = Attribute(
6 "Does this target support merge proposals at all?")
7
8+ supports_code_imports = Attribute(
9+ "Does this target support code imports at all?")
10+
11 def areBranchesMergeable(other_target):
12 """Are branches from other_target mergeable into this target."""
13
14@@ -113,3 +116,18 @@
15
16 def getBugTask(bug):
17 """Get the BugTask for a given bug related to the branch target."""
18+
19+ def newCodeImport(registrant, branch_name, rcs_type, url=None,
20+ cvs_root=None, cvs_module=None):
21+ """Create a new code import for this target.
22+
23+ :param registrant: the `IPerson` who should be recorded as creating
24+ the import and will own the resulting branch.
25+ :param branch_name: the name the resulting branch should have.
26+ :param rcs_type: the type of the foreign VCS.
27+ :param url: the url to import from if the import isn't CVS.
28+ :param cvs_root: if the import is from CVS the CVSROOT to import from.
29+ :param cvs_module: if the import is from CVS the module to import.
30+ :returns: an `ICodeImport`.
31+ :raises AssertionError: if supports_code_imports is False.
32+ """
33
34=== modified file 'lib/lp/code/model/branchtarget.py'
35--- lib/lp/code/model/branchtarget.py 2010-01-05 08:18:23 +0000
36+++ lib/lp/code/model/branchtarget.py 2010-04-02 02:44:29 +0000
37@@ -19,6 +19,7 @@
38 from lp.code.interfaces.branchcollection import IAllBranches
39 from lp.code.interfaces.branchtarget import (
40 check_default_stacked_on, IBranchTarget)
41+from lp.code.interfaces.codeimport import ICodeImportSet
42 from lp.registry.interfaces.pocket import PackagePublishingPocket
43 from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
44
45@@ -36,6 +37,12 @@
46 def __ne__(self, other):
47 return self.context != other.context
48
49+ def newCodeImport(self, registrant, branch_name, rcs_type, url=None,
50+ cvs_root=None, cvs_module=None):
51+ return getUtility(ICodeImportSet).new(
52+ registrant, self, branch_name, rcs_type, url=url,
53+ cvs_root=cvs_root, cvs_module=cvs_module)
54+
55
56 class PackageBranchTarget(_BaseBranchTarget):
57 implements(IBranchTarget)
58@@ -95,6 +102,11 @@
59 """See `IBranchTarget`."""
60 return True
61
62+ @property
63+ def supports_code_imports(self):
64+ """See `IBranchTarget`."""
65+ return False
66+
67 def areBranchesMergeable(self, other_target):
68 """See `IBranchTarget`."""
69 # Branches are mergable into a PackageTarget if the source package
70@@ -182,6 +194,11 @@
71 """See `IBranchTarget`."""
72 return False
73
74+ @property
75+ def supports_code_imports(self):
76+ """See `IBranchTarget`."""
77+ return False
78+
79 def areBranchesMergeable(self, other_target):
80 """See `IBranchTarget`."""
81 return False
82@@ -258,6 +275,11 @@
83 """See `IBranchTarget`."""
84 return True
85
86+ @property
87+ def supports_code_imports(self):
88+ """See `IBranchTarget`."""
89+ return True
90+
91 def areBranchesMergeable(self, other_target):
92 """See `IBranchTarget`."""
93 # Branches are mergable into a PackageTarget if the source package
94@@ -311,6 +333,11 @@
95 """See `IBranchTarget`."""
96 return self.productseries
97
98+ @property
99+ def supports_code_imports(self):
100+ """See `IBranchTarget`."""
101+ return False
102+
103
104 def get_canonical_url_data_for_target(branch_target):
105 """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
106
107=== modified file 'lib/lp/code/model/codeimport.py'
108--- lib/lp/code/model/codeimport.py 2010-04-02 02:44:28 +0000
109+++ lib/lp/code/model/codeimport.py 2010-04-02 02:44:29 +0000
110@@ -225,6 +225,8 @@
111 review_status = CodeImportReviewStatus.REVIEWED
112 else:
113 review_status = CodeImportReviewStatus.NEW
114+ if not target.supports_code_imports:
115+ raise AssertionError("%r doesn't support code imports" % target)
116 # Create the branch for the CodeImport.
117 namespace = target.getNamespace(registrant)
118 import_branch = namespace.createBranch(
119
120=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
121--- lib/lp/code/model/tests/test_branchtarget.py 2010-03-18 15:39:58 +0000
122+++ lib/lp/code/model/tests/test_branchtarget.py 2010-04-02 02:44:29 +0000
123@@ -14,8 +14,9 @@
124 check_default_stacked_on,
125 PackageBranchTarget, PersonBranchTarget, ProductBranchTarget,
126 ProductSeriesBranchTarget)
127-from lp.code.enums import BranchType
128+from lp.code.enums import BranchType, RevisionControlSystems
129 from lp.code.interfaces.branchtarget import IBranchTarget
130+from lp.code.interfaces.codeimport import ICodeImport
131 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
132 from lp.registry.interfaces.pocket import PackagePublishingPocket
133 from canonical.launchpad.webapp import canonical_url
134@@ -176,6 +177,16 @@
135 ubuntu_branches.teamowner)
136 self.assertEqual(branch, self.target.default_merge_target)
137
138+ def test_doesnt_support_code_imports(self):
139+ self.assertFalse(self.target.supports_code_imports)
140+
141+ def test_creating_code_import_fails(self):
142+ self.assertRaises(
143+ AssertionError, self.target.newCodeImport,
144+ self.factory.makePerson(),
145+ self.factory.getUniqueString("name-"),
146+ RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
147+
148
149 class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
150
151@@ -258,6 +269,16 @@
152 self.target._retargetBranch(removeSecurityProxy(branch))
153 self.assertEqual(self.target, branch.target)
154
155+ def test_doesnt_support_code_imports(self):
156+ self.assertFalse(self.target.supports_code_imports)
157+
158+ def test_creating_code_import_fails(self):
159+ self.assertRaises(
160+ AssertionError, self.target.newCodeImport,
161+ self.factory.makePerson(),
162+ self.factory.getUniqueString("name-"),
163+ RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
164+
165
166 class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
167
168@@ -364,6 +385,23 @@
169 setattr, self.original.development_focus, 'branch', branch)
170 self.assertEqual(branch, self.target.default_merge_target)
171
172+ def test_supports_code_imports(self):
173+ self.assertTrue(self.target.supports_code_imports)
174+
175+ def test_creating_code_import_succeeds(self):
176+ target_url = self.factory.getUniqueURL()
177+ branch_name = self.factory.getUniqueString("name-")
178+ owner = self.factory.makePerson()
179+ code_import = self.target.newCodeImport(
180+ owner, branch_name, RevisionControlSystems.GIT, url=target_url)
181+ code_import = removeSecurityProxy(code_import)
182+ self.assertProvides(code_import, ICodeImport)
183+ self.assertEqual(target_url, code_import.url)
184+ self.assertEqual(branch_name, code_import.branch.name)
185+ self.assertEqual(owner, code_import.registrant)
186+ self.assertEqual(owner, code_import.branch.owner)
187+ self.assertEqual(self.target, code_import.branch.target)
188+
189
190 class TestProductSeriesBranchTarget(TestCaseWithFactory):
191
192@@ -378,6 +416,16 @@
193 target = IBranchTarget(self.original)
194 self.assertIsInstance(target, ProductSeriesBranchTarget)
195
196+ def test_doesnt_support_code_imports(self):
197+ self.assertFalse(self.target.supports_code_imports)
198+
199+ def test_creating_code_import_fails(self):
200+ self.assertRaises(
201+ AssertionError, self.target.newCodeImport,
202+ self.factory.makePerson(),
203+ self.factory.getUniqueString("name-"),
204+ RevisionControlSystems.GIT, url=self.factory.getUniqueURL())
205+
206
207 class TestCheckDefaultStackedOnBranch(TestCaseWithFactory):
208 """Only certain branches are allowed to be default stacked-on branches."""
209
210=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
211--- lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:44:28 +0000
212+++ lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:44:29 +0000
213@@ -120,9 +120,20 @@
214 self.assertEqual(
215 CodeImportReviewStatus.REVIEWED,
216 code_import.review_status)
217- # No job is created for the import.
218+ # A job is created for the import.
219 self.assertIsNot(None, code_import.import_job)
220
221+ def test_junk_code_import_rejected(self):
222+ """You are not allowed to create code imports targetting +junk."""
223+ registrant = self.factory.makePerson()
224+ self.assertRaises(AssertionError, CodeImportSet().new,
225+ registrant=registrant,
226+ target=IBranchTarget(registrant),
227+ branch_name='imported',
228+ rcs_type=RevisionControlSystems.HG,
229+ url=self.factory.getUniqueURL(),
230+ review_status=None)
231+
232
233 class TestCodeImportDeletion(TestCaseWithFactory):
234 """Test the deletion of CodeImports."""