Merge lp:~jcsackett/launchpad/deprecate-official_codehosting into lp:launchpad

Proposed by j.c.sackett
Status: Merged
Approved by: j.c.sackett
Approved revision: no longer in the source branch.
Merged at revision: 11484
Proposed branch: lp:~jcsackett/launchpad/deprecate-official_codehosting
Merge into: lp:launchpad
Diff against target: 330 lines (+72/-30)
8 files modified
lib/lp/registry/adapters.py (+18/-2)
lib/lp/registry/browser/distribution.py (+2/-1)
lib/lp/registry/browser/pillar.py (+16/-7)
lib/lp/registry/browser/productseries.py (+11/-3)
lib/lp/registry/browser/tests/pillar-views.txt (+8/-8)
lib/lp/registry/browser/tests/productseries-views.txt (+4/-4)
lib/lp/registry/configure.zcml (+7/-0)
lib/lp/registry/stories/product/xx-product-launchpad-usage.txt (+6/-5)
To merge this branch: bzr merge lp:~jcsackett/launchpad/deprecate-official_codehosting
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+33953@code.launchpad.net

Commit message

Replaces use of official_codehosting with the codehosting usage enum where possible.

Description of the change

= Summary =

Replaces use of official_codehosting with codehosting_attributes where possible.

== Proposed fix ==

Where official_codehosting is used, replace it with codehosting_usage and the ServiceUsage enums.

== Pre-implementation notes ==

Spoke with Curtis.

== Implementation details ==

As in Proposed fix.

== Tests ==

bin/test -vvc -t productseries-views.txt
bin/test -vvc -t pillar-views.txt
bin/test -vvc -t distribution-views.txt

== Demo and Q/A ==

Check out a project, product and distribution on launchpad.dev.

Everything should function as before--this shouldn't have introduced
any visible changes.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/adapters.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/browser/distribution.py
  lib/lp/registry/browser/pillar.py
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/pillar-views.txt
  lib/lp/registry/browser/tests/productseries-views.txt

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

This branch looks nice Jon. What would you think of creating some helper functions like:

uses_Launchpad(thing)

You could then collapse this:

175 + if self.codehosting_usage == ServiceUsage.LAUNCHPAD:
176 + configured = True
177 + else:
178 + configured = False
179 return [dict(link=set_branch,
181 + configured=configured)]

to:

179 return [dict(link=set_branch,
181 + configured=uses_launchpad(self.codehosting_usage)

I guess you could do the same with:

configured = (self.codehosting_usage == ServiceUsage.LAUNCHPAD)

with less overhead.

review: Approve (code)
Revision history for this message
j.c.sackett (jcsackett) wrote :

Brad,

I actually deliberately expanded some clauses into the if constructs because I found them easier to parse.

I'm not opposed to a helper function, since the inside of that would be fairly clear, but assigning a boolean
expression to a variable is sort of difficult to scan in a large block of code.

Thoughts?

On Aug 27, 2010, at 4:49 PM, Brad Crittenden wrote:

> Review: Approve code
> This branch looks nice Jon. What would you think of creating some helper functions like:
>
> uses_Launchpad(thing)
>
> You could then collapse this:
>
> 175 + if self.codehosting_usage == ServiceUsage.LAUNCHPAD:
> 176 + configured = True
> 177 + else:
> 178 + configured = False
> 179 return [dict(link=set_branch,
> 181 + configured=configured)]
>
> to:
>
> 179 return [dict(link=set_branch,
> 181 + configured=uses_launchpad(self.codehosting_usage)
>
> I guess you could do the same with:
>
> configured = (self.codehosting_usage == ServiceUsage.LAUNCHPAD)
>
> with less overhead.
> --
> https://code.launchpad.net/~jcsackett/launchpad/deprecate-official_codehosting/+merge/33953
> Your team Launchpad code reviewers from Canonical is subscribed to branch lp:launchpad/devel.
>
> --
> launchpad-reviews mailing list
> <email address hidden>
> https://lists.ubuntu.com/mailman/listinfo/launchpad-reviews

Revision history for this message
Brad Crittenden (bac) wrote :

On Aug 27, 2010, at 19:25, "j.c.sackett" <email address hidden> wrote:

> Brad,
>
> I actually deliberately expanded some clauses into the if constructs because I found them easier to parse.
>
> I'm not opposed to a helper function, since the inside of that would be fairly clear, but assigning a boolean
> expression to a variable is sort of difficult to scan in a large block of code.
>
> Thoughts?

What you have now is fine to land. If we find ourselves repeating that logic elsewhere then we can create a helper.

Brad

>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/adapters.py'
--- lib/lp/registry/adapters.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/adapters.py 2010-08-31 14:49:44 +0000
@@ -7,16 +7,25 @@
77
8__all__ = [8__all__ = [
9 'distroseries_to_launchpadusage',9 'distroseries_to_launchpadusage',
10 'distroseries_to_serviceusage',
10 'PollSubset',11 'PollSubset',
11 'productseries_to_product',12 'productseries_to_product',
12 ]13 ]
1314
1415
15from zope.component import getUtility16from zope.component import (
17 adapter,
18 getUtility,
19 )
16from zope.component.interfaces import ComponentLookupError20from zope.component.interfaces import ComponentLookupError
17from zope.interface import implements21from zope.interface import (
22 implementer,
23 implements,
24 )
1825
19from canonical.launchpad.webapp.interfaces import ILaunchpadPrincipal26from canonical.launchpad.webapp.interfaces import ILaunchpadPrincipal
27from lp.app.interfaces.launchpad import IServiceUsage
28from lp.registry.interfaces.distroseries import IDistroSeries
20from lp.registry.interfaces.poll import (29from lp.registry.interfaces.poll import (
21 IPollSet,30 IPollSet,
22 IPollSubset,31 IPollSubset,
@@ -25,6 +34,13 @@
25 )34 )
2635
2736
37@implementer(IServiceUsage)
38@adapter(IDistroSeries)
39def distroseries_to_serviceusage(distroseries):
40 """Adapts `IDistroSeries` object to `IServiceUsage`."""
41 return distroseries.distribution
42
43
28def distroseries_to_launchpadusage(distroseries):44def distroseries_to_launchpadusage(distroseries):
29 """Adapts `IDistroSeries` object to `ILaunchpadUsage`."""45 """Adapts `IDistroSeries` object to `ILaunchpadUsage`."""
30 return distroseries.distribution46 return distroseries.distribution
3147
=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py 2010-08-26 22:44:30 +0000
+++ lib/lp/registry/browser/distribution.py 2010-08-31 14:49:44 +0000
@@ -80,6 +80,7 @@
80 QuestionTargetFacetMixin,80 QuestionTargetFacetMixin,
81 QuestionTargetTraversalMixin,81 QuestionTargetTraversalMixin,
82 )82 )
83from lp.app.enums import ServiceUsage
83from lp.app.errors import NotFoundError84from lp.app.errors import NotFoundError
84from lp.blueprints.browser.specificationtarget import (85from lp.blueprints.browser.specificationtarget import (
85 HasSpecificationsMenuMixin,86 HasSpecificationsMenuMixin,
@@ -133,7 +134,7 @@
133 url = canonical_url(self.context, rootsite='bugs')134 url = canonical_url(self.context, rootsite='bugs')
134 uses.append(href_template % (url, 'Bug Tracking'))135 uses.append(href_template % (url, 'Bug Tracking'))
135 if IProduct.providedBy(self.context):136 if IProduct.providedBy(self.context):
136 if self.context.official_codehosting:137 if self.context.codehosting_usage == ServiceUsage.LAUNCHPAD:
137 url = canonical_url(self.context, rootsite='code')138 url = canonical_url(self.context, rootsite='code')
138 uses.append(href_template % (url, 'Branches'))139 uses.append(href_template % (url, 'Branches'))
139 if self.context.official_rosetta:140 if self.context.official_rosetta:
140141
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/pillar.py 2010-08-31 14:49:44 +0000
@@ -31,6 +31,8 @@
31 nearest,31 nearest,
32 )32 )
33from canonical.launchpad.webapp.tales import MenuAPI33from canonical.launchpad.webapp.tales import MenuAPI
34from lp.app.enums import ServiceUsage
35from lp.app.interfaces.launchpad import IServiceUsage
34from lp.registry.browser.structuralsubscription import (36from lp.registry.browser.structuralsubscription import (
35 StructuralSubscriptionMenuMixin,37 StructuralSubscriptionMenuMixin,
36 )38 )
@@ -73,9 +75,16 @@
73 enabled=self.pillar.official_rosetta)75 enabled=self.pillar.official_rosetta)
7476
75 def submit_code(self):77 def submit_code(self):
78 if self.pillar.codehosting_usage in [
79 ServiceUsage.LAUNCHPAD,
80 ServiceUsage.EXTERNAL,
81 ]:
82 enabled = True
83 else:
84 enabled = False
76 return Link(85 return Link(
77 '+addbranch', 'Submit code', site='code', icon='code',86 '+addbranch', 'Submit code', site='code', icon='code',
78 enabled=self.pillar.official_codehosting)87 enabled=enabled)
7988
80 def register_blueprint(self):89 def register_blueprint(self):
81 return Link(90 return Link(
@@ -96,19 +105,20 @@
96 self.official_answers = False105 self.official_answers = False
97 self.official_blueprints = False106 self.official_blueprints = False
98 self.official_rosetta = False107 self.official_rosetta = False
99 self.official_codehosting = False108 self.codehosting_usage = ServiceUsage.UNKNOWN
100 pillar = nearest(self.context, IPillar)109 pillar = nearest(self.context, IPillar)
101 if IProjectGroup.providedBy(pillar):110 if IProjectGroup.providedBy(pillar):
102 for product in pillar.products:111 for product in pillar.products:
103 self._set_official_launchpad(product)112 self._set_official_launchpad(product)
104 # Project groups do not support submit code, override the113 # Project groups do not support submit code, override the
105 # default.114 # default.
106 self.official_codehosting = False115 self.codehosting_usage = ServiceUsage.NOT_APPLICABLE
107 else:116 else:
108 self._set_official_launchpad(pillar)117 self._set_official_launchpad(pillar)
109 if IDistroSeries.providedBy(self.context):118 if IDistroSeries.providedBy(self.context):
110 self.official_answers = False119 self.official_answers = False
111 self.official_codehosting = False120 distribution = self.context.distribution
121 self.codehosting_usage = distribution.codehosting_usage
112 elif IDistributionSourcePackage.providedBy(self.context):122 elif IDistributionSourcePackage.providedBy(self.context):
113 self.official_blueprints = False123 self.official_blueprints = False
114 self.official_rosetta = False124 self.official_rosetta = False
@@ -128,8 +138,7 @@
128 self.official_blueprints = True138 self.official_blueprints = True
129 if pillar.official_rosetta:139 if pillar.official_rosetta:
130 self.official_rosetta = True140 self.official_rosetta = True
131 if pillar.official_codehosting:141 self.codehosting_usage = IServiceUsage(pillar).codehosting_usage
132 self.official_codehosting = True
133142
134 @property143 @property
135 def has_involvement(self):144 def has_involvement(self):
@@ -137,7 +146,7 @@
137 return (146 return (
138 self.official_malone or self.official_answers147 self.official_malone or self.official_answers
139 or self.official_blueprints or self.official_rosetta148 or self.official_blueprints or self.official_rosetta
140 or self.official_codehosting)149 or self.codehosting_usage == ServiceUsage.LAUNCHPAD)
141150
142 @property151 @property
143 def enabled_links(self):152 def enabled_links(self):
144153
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py 2010-08-23 04:48:17 +0000
+++ lib/lp/registry/browser/productseries.py 2010-08-31 14:49:44 +0000
@@ -83,6 +83,7 @@
83from canonical.launchpad.webapp.tales import MenuAPI83from canonical.launchpad.webapp.tales import MenuAPI
84from canonical.widgets.itemswidgets import LaunchpadRadioWidget84from canonical.widgets.itemswidgets import LaunchpadRadioWidget
85from canonical.widgets.textwidgets import StrippedTextWidget85from canonical.widgets.textwidgets import StrippedTextWidget
86from lp.app.enums import ServiceUsage
86from lp.app.errors import (87from lp.app.errors import (
87 NotFoundError,88 NotFoundError,
88 UnexpectedFormData,89 UnexpectedFormData,
@@ -237,7 +238,7 @@
237 def submit_code(self):238 def submit_code(self):
238 target = canonical_url(239 target = canonical_url(
239 self.pillar, view_name='+addbranch', rootsite='code')240 self.pillar, view_name='+addbranch', rootsite='code')
240 enabled = self.view.official_codehosting241 enabled = self.view.codehosting_usage == ServiceUsage.LAUNCHPAD
241 return Link(242 return Link(
242 target, 'Submit code', icon='code', enabled=enabled)243 target, 'Submit code', icon='code', enabled=enabled)
243244
@@ -251,7 +252,10 @@
251252
252 def __init__(self, context, request):253 def __init__(self, context, request):
253 super(ProductSeriesInvolvementView, self).__init__(context, request)254 super(ProductSeriesInvolvementView, self).__init__(context, request)
254 self.official_codehosting = self.context.branch is not None255 if self.context.branch is not None:
256 self.codehosting_usage = ServiceUsage.LAUNCHPAD
257 else:
258 self.codehosting_usage = ServiceUsage.UNKNOWN
255 self.official_answers = False259 self.official_answers = False
256260
257 @property261 @property
@@ -260,8 +264,12 @@
260 series_menu = MenuAPI(self.context).overview264 series_menu = MenuAPI(self.context).overview
261 set_branch = series_menu['set_branch']265 set_branch = series_menu['set_branch']
262 set_branch.text = 'Configure series branch'266 set_branch.text = 'Configure series branch'
267 if self.codehosting_usage == ServiceUsage.LAUNCHPAD:
268 configured = True
269 else:
270 configured = False
263 return [dict(link=set_branch,271 return [dict(link=set_branch,
264 configured=self.official_codehosting)]272 configured=configured)]
265273
266274
267class ProductSeriesOverviewMenu(275class ProductSeriesOverviewMenu(
268276
=== modified file 'lib/lp/registry/browser/tests/pillar-views.txt'
--- lib/lp/registry/browser/tests/pillar-views.txt 2010-08-19 20:11:11 +0000
+++ lib/lp/registry/browser/tests/pillar-views.txt 2010-08-31 14:49:44 +0000
@@ -39,8 +39,8 @@
39 False39 False
40 >>> view.official_blueprints40 >>> view.official_blueprints
41 False41 False
42 >>> view.official_codehosting42 >>> view.codehosting_usage.name
43 False43 'NOT_APPLICABLE'
4444
45The view provides a list of enabled links that is rendered by the template.45The view provides a list of enabled links that is rendered by the template.
4646
@@ -200,23 +200,23 @@
200 >>> product.official_codehosting200 >>> product.official_codehosting
201 False201 False
202 >>> view = create_view(product, '+get-involved')202 >>> view = create_view(product, '+get-involved')
203 >>> view.official_codehosting203 >>> view.codehosting_usage.name
204 False204 'UNKNOWN'
205205
206 >>> product.development_focus.branch = factory.makeBranch(206 >>> product.development_focus.branch = factory.makeBranch(
207 ... product=product)207 ... product=product)
208 >>> product.official_codehosting208 >>> product.official_codehosting
209 True209 True
210 >>> view = create_view(product, '+get-involved')210 >>> view = create_view(product, '+get-involved')
211 >>> view.official_codehosting211 >>> view.codehosting_usage.name
212 True212 'LAUNCHPAD'
213213
214Project groups cannot make links to register a branch, so214Project groups cannot make links to register a branch, so
215official_codehosting is always false.215official_codehosting is always false.
216216
217 >>> view = create_view(project_group, '+get-involved')217 >>> view = create_view(project_group, '+get-involved')
218 >>> view.official_codehosting218 >>> view.codehosting_usage.name
219 False219 'NOT_APPLICABLE'
220220
221DistroSeries can use this view. The distribution is used to set the links.221DistroSeries can use this view. The distribution is used to set the links.
222222
223223
=== modified file 'lib/lp/registry/browser/tests/productseries-views.txt'
--- lib/lp/registry/browser/tests/productseries-views.txt 2010-08-23 04:48:17 +0000
+++ lib/lp/registry/browser/tests/productseries-views.txt 2010-08-31 14:49:44 +0000
@@ -39,8 +39,8 @@
39 True39 True
40 >>> print view.official_rosetta40 >>> print view.official_rosetta
41 True41 True
42 >>> print view.official_codehosting42 >>> print view.codehosting_usage.name
43 False43 UNKNOWN
44 >>> for link in view.enabled_links:44 >>> for link in view.enabled_links:
45 ... print link.url45 ... print link.url
46 http://bugs.launchpad.dev/app/simple/+filebug46 http://bugs.launchpad.dev/app/simple/+filebug
@@ -54,8 +54,8 @@
5454
55 >>> series.branch = factory.makeBranch()55 >>> series.branch = factory.makeBranch()
56 >>> view = create_view(series, '+get-involved')56 >>> view = create_view(series, '+get-involved')
57 >>> print view.official_codehosting57 >>> print view.codehosting_usage.name
58 True58 LAUNCHPAD
59 >>> for link in view.enabled_links:59 >>> for link in view.enabled_links:
60 ... print link.url60 ... print link.url
61 http://bugs.launchpad.dev/app/simple/+filebug61 http://bugs.launchpad.dev/app/simple/+filebug
6262
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-08-23 03:25:20 +0000
+++ lib/lp/registry/configure.zcml 2010-08-31 14:49:44 +0000
@@ -171,6 +171,8 @@
171 factory="lp.registry.adapters.distroseries_to_launchpadusage"171 factory="lp.registry.adapters.distroseries_to_launchpadusage"
172 permission="zope.Public"/>172 permission="zope.Public"/>
173 <adapter173 <adapter
174 factory="lp.registry.adapters.distroseries_to_serviceusage" />
175 <adapter
174 provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"176 provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
175 for="lp.registry.interfaces.distroseries.IDistroSeries"177 for="lp.registry.interfaces.distroseries.IDistroSeries"
176 factory="lp.registry.browser.distroseries.DistroSeriesBreadcrumb"178 factory="lp.registry.browser.distroseries.DistroSeriesBreadcrumb"
@@ -1379,6 +1381,11 @@
1379 factory="lp.registry.adapters.productseries_to_product"1381 factory="lp.registry.adapters.productseries_to_product"
1380 permission="zope.Public"/>1382 permission="zope.Public"/>
1381 <adapter1383 <adapter
1384 provides="lp.app.interfaces.launchpad.IServiceUsage"
1385 for="lp.registry.interfaces.productseries.IProductSeries"
1386 factory="lp.registry.adapters.productseries_to_product"
1387 permission="zope.Public"/>
1388 <adapter
1382 provides="lp.app.interfaces.launchpad.ILaunchpadUsage"1389 provides="lp.app.interfaces.launchpad.ILaunchpadUsage"
1383 for="lp.registry.interfaces.productseries.IProductSeries"1390 for="lp.registry.interfaces.productseries.IProductSeries"
1384 factory="lp.registry.adapters.productseries_to_product"1391 factory="lp.registry.adapters.productseries_to_product"
13851392
=== modified file 'lib/lp/registry/stories/product/xx-product-launchpad-usage.txt'
--- lib/lp/registry/stories/product/xx-product-launchpad-usage.txt 2010-08-25 00:01:57 +0000
+++ lib/lp/registry/stories/product/xx-product-launchpad-usage.txt 2010-08-31 14:49:44 +0000
@@ -47,7 +47,8 @@
47 >>> registrant_browser.getControl('Somewhere else').selected = True47 >>> registrant_browser.getControl('Somewhere else').selected = True
48 >>> registrant_browser.getControl('Change').click()48 >>> registrant_browser.getControl('Change').click()
4949
50We'll also set it as officially using codehosting.50We'll also set up a development focus branch. Because it's a mirrored
51branch, codehosting won't show up as a service the product uses.
5152
52 >>> registrant_browser.getLink('Configure project branch').click()53 >>> registrant_browser.getLink('Configure project branch').click()
53 >>> registrant_browser.getControl(name='field.branch_location').value = (54 >>> registrant_browser.getControl(name='field.branch_location').value = (
@@ -89,7 +90,7 @@
8990
90 >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')91 >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')
91 >>> print extract_text(uses)92 >>> print extract_text(uses)
92 Uses Launchpad for: Branches and Translations.93 Uses Launchpad for: Translations.
9394
94Blueprints can be enabled too, though it isn't easy.95Blueprints can be enabled too, though it isn't easy.
9596
@@ -110,12 +111,12 @@
110111
111112
112On the product page, we can see that the product doesn't use any bug113On the product page, we can see that the product doesn't use any bug
113tracker, not even Launchpad, but does use other apps.114tracker, not even Launchpad, but does use Translations.
114115
115 >>> registrant_browser.open('http://launchpad.dev/firefox')116 >>> registrant_browser.open('http://launchpad.dev/firefox')
116 >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')117 >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')
117 >>> print extract_text(uses)118 >>> print extract_text(uses)
118 Uses Launchpad for: Blueprints, Branches, and Translations.119 Uses Launchpad for: Blueprints and Translations.
119120
120Setting a usage to external is reflected in the "Uses" message.121Setting a usage to external is reflected in the "Uses" message.
121Change translations to external.122Change translations to external.
@@ -127,7 +128,7 @@
127128
128 >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')129 >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')
129 >>> print extract_text(uses)130 >>> print extract_text(uses)
130 Uses Launchpad for: Blueprints and Branches.131 Uses Launchpad for: Blueprints.
131132
132133
133Tracking bugs by email134Tracking bugs by email