Launchpad itself

Merge lp://staging/~julian-edwards/launchpad/mechanical-changes-8 into lp://staging/launchpad

  1. mechanical-changes-8
  2. Merge into devel
Proposed by Julian Edwards
Status: Merged
Approved by: Barry Warsaw
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp://staging/~julian-edwards/launchpad/mechanical-changes-8
Merge into: lp://staging/launchpad
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~julian-edwards/launchpad/mechanical-changes-8
Reviewer Review Type Date Requested Status
Barry Warsaw (community) Approve
Review via email: mp+11196@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

= Summary =
3.0 mechanical changes to sourcepackage builds lists

== Implementation details ==
* SourcePackageView no longer inherits from BuildRecords view
* New view SourcePackageBuildsView for the converted page so that it better
encapsulates the build-related stuff, and has a separate page title.
* Very trivial template change!

As a bonus, I fixed the headings on the distroseriesbinarypackage-index.pt
that I committed a couple of hours ago, the h1 and h2 were the wrong way
around (and nobody spotted it!).

== Tests ==
bin/test -cvvt xx-builds-pages

== Demo and Q/A ==
For the builds change:
https://launchpad.dev/ubuntu/warty/+source/mozilla-firefox/+builds

For the h1/h2 swap:
https://launchpad.dev/ubuntu/warty/+package/mozilla-firefox/
or
https://dogfood.launchpad.net/ubuntu/hardy/+package/kmail

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/browser/sourcepackage.py
  lib/lp/soyuz/templates/sourcepackage-builds.pt
  lib/lp/soyuz/browser/sourcepackagebuilds.py
  lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt
  lib/lp/soyuz/browser/configure.zcml

Revision history for this message
Barry Warsaw (barry) wrote :
Download full text (7.3 KiB)

On Sep 04, 2009, at 01:15 PM, Julian Edwards wrote:

>= Summary =
>3.0 mechanical changes to sourcepackage builds lists

Awesome, thanks for working on these. 3.0 is really shaping up nicely!

>
>== Implementation details ==
>* SourcePackageView no longer inherits from BuildRecords view
>* New view SourcePackageBuildsView for the converted page so that it better
>encapsulates the build-related stuff, and has a separate page title.

I'm finding that refactoring our view classes really helps out. I think the
new header rules will encourage more of that. Ultimately I think it's better
to use mixins and subclasses more and stop overloading our view classes with
tons of functionality.

>* Very trivial template change!
>
>As a bonus, I fixed the headings on the distroseriesbinarypackage-index.pt
>that I committed a couple of hours ago, the h1 and h2 were the wrong way
>around (and nobody spotted it!).

We'll see about this one. :)

>
>== Tests ==
>bin/test -cvvt xx-builds-pages
>
>== Demo and Q/A ==
>For the builds change:
>https://launchpad.dev/ubuntu/warty/+source/mozilla-firefox/+builds
>
>For the h1/h2 swap:
>https://launchpad.dev/ubuntu/warty/+package/mozilla-firefox/

I took a look at this latter page. Once my branch
lp:~barry/launchpad/417089-headings is reviewed and landed, I have some
suggestions for making this page conform to our really-it's-final-this-time
header and title rules. The changes should be simple enough that you can
update the +builds page too, and as I mentioned in IRC, I will give you an
rs=me for a follow up branch to fix this. We may want to give a blanket rs
for any other branches that fix pages to the new rules.

Here's the diff of the changes I had to make.

=== modified file 'lib/lp/soyuz/browser/distroseriesbinarypackage.py'
--- lib/lp/soyuz/browser/distroseriesbinarypackage.py 2009-09-03 16:38:19 +0000
+++ lib/lp/soyuz/browser/distroseriesbinarypackage.py 2009-09-04 15:44:12 +0000
@@ -51,6 +51,5 @@
         self.context = context
         self.request = request

- @property
- def page_title(self):
- return smartquote(self.context.title)
+ def label(self):
+ return self.context.title

=== modified file 'lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt'
--- lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt 2009-09-04 12:43:13 +0000
+++ lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt 2009-09-04 15:32:31 +0000
@@ -8,14 +8,8 @@
 >

 <body>
- <metal:heading fill-slot="heading">
- <h2 tal:content="view/page_title"/>
- </metal:heading>
-
 <div metal:fill-slot="main">

- <h1 tal:content="context/summary"/>
-
   <tal:description replace="structure context/description/fmt:text-to-html">
       Package Description
   </tal:description>

We noticed a few things. First, the def label() should not be necessary.
I'll update my branch so that the base-layout falls back to view.context.title
when view.label is not defined. Second, by default base-layout cgi.escapes
the label, but it should also smartquote it. Third, I think you can basically
get rid of DistroSeriesBinaryPackageView. Once my branch lands, there'll be
no reason not to just use LaunchpadView. Remo...

Read more...

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/sourcepackage.py'
2--- lib/lp/registry/browser/sourcepackage.py 2009-09-04 07:29:56 +0000
3+++ lib/lp/registry/browser/sourcepackage.py 2009-09-04 12:32:58 +0000
4@@ -19,7 +19,6 @@
5
6 from canonical.launchpad import helpers
7 from lp.bugs.browser.bugtask import BugTargetTraversalMixin
8-from lp.soyuz.browser.build import BuildRecordsView
9 from canonical.launchpad.browser.packagerelationship import (
10 relationship_builder)
11 from lp.answers.browser.questiontarget import (
12@@ -140,7 +139,8 @@
13 self.next_url = canonical_url(self.context)
14
15
16-class SourcePackageView(BuildRecordsView):
17+class SourcePackageView:
18+ """A view for (distro series) source packages."""
19
20 def initialize(self):
21 # lets add a widget for the product series to which this package is
22@@ -245,17 +245,3 @@
23 @property
24 def potemplates(self):
25 return list(self.context.getCurrentTranslationTemplates())
26-
27- @property
28- def search_name(self):
29- return False
30-
31- @property
32- def default_build_state(self):
33- """Default build state for sourcepackage builds.
34-
35- This overrides the default that is set on BuildRecordsView."""
36- # None maps to "all states". The reason we display all states on
37- # this page is because it's unlikely that there will be so
38- # many builds that the listing will be overwhelming.
39- return None
40
41=== modified file 'lib/lp/soyuz/browser/configure.zcml'
42--- lib/lp/soyuz/browser/configure.zcml 2009-08-21 18:05:01 +0000
43+++ lib/lp/soyuz/browser/configure.zcml 2009-09-04 12:06:54 +0000
44@@ -722,18 +722,24 @@
45 facet="overview"
46 template="../templates/sourcepackage-packaging.pt"/>
47 <browser:page
48- name="+builds"
49- facet="overview"
50- template="../templates/sourcepackage-builds.pt"/>
51- <browser:page
52- name="+builds-list"
53- template="../templates/builds-list.pt"/>
54- <browser:page
55 name="+changelog"
56 facet="overview"
57 template="../templates/sourcepackage-changelog.pt"/>
58 </browser:pages>
59 <browser:pages
60+ for="lp.registry.interfaces.sourcepackage.ISourcePackage"
61+ permission="zope.Public"
62+ class=
63+ "lp.soyuz.browser.sourcepackagebuilds.SourcePackageBuildsView">
64+ <browser:page
65+ name="+builds-list"
66+ template="../templates/builds-list.pt"/>
67+ <browser:page
68+ name="+builds"
69+ facet="overview"
70+ template="../templates/sourcepackage-builds.pt"/>
71+ </browser:pages>
72+ <browser:pages
73 for="lp.registry.interfaces.person.IPerson"
74 permission="zope.Public"
75 class="lp.registry.browser.person.PersonView">
76
77=== added file 'lib/lp/soyuz/browser/sourcepackagebuilds.py'
78--- lib/lp/soyuz/browser/sourcepackagebuilds.py 1970-01-01 00:00:00 +0000
79+++ lib/lp/soyuz/browser/sourcepackagebuilds.py 2009-09-04 12:22:22 +0000
80@@ -0,0 +1,37 @@
81+# Copyright 2009 Canonical Ltd. This software is licensed under the
82+# GNU Affero General Public License version 3 (see the file LICENSE).
83+
84+"""Browser views for source package builds."""
85+
86+__metaclass__ = type
87+
88+__all__ = [
89+ 'SourcePackageBuildsView',
90+ ]
91+
92+from lp.soyuz.browser.build import BuildRecordsView
93+from canonical.lazr.utils import smartquote
94+
95+
96+class SourcePackageBuildsView(BuildRecordsView):
97+ """A view for (distro series) source package builds."""
98+
99+ @property
100+ def page_title(self):
101+ return smartquote("Builds for " + self.context.title)
102+
103+ @property
104+ def search_name(self):
105+ """Direct the builds-list template to omit the name search field."""
106+ return False
107+
108+ @property
109+ def default_build_state(self):
110+ """Default build state for sourcepackage builds.
111+
112+ This overrides the default that is set on BuildRecordsView."""
113+ # None maps to "all states". The reason we display all states on
114+ # this page is because it's unlikely that there will be so
115+ # many builds that the listing will be overwhelming.
116+ return None
117+
118
119=== modified file 'lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt'
120--- lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt 2009-09-03 16:38:19 +0000
121+++ lib/lp/soyuz/templates/distroseriesbinarypackage-index.pt 2009-09-04 12:43:13 +0000
122@@ -9,12 +9,12 @@
123
124 <body>
125 <metal:heading fill-slot="heading">
126- <h1 tal:content="view/page_title"/>
127+ <h2 tal:content="view/page_title"/>
128 </metal:heading>
129
130 <div metal:fill-slot="main">
131
132- <h2 tal:content="context/summary"/>
133+ <h1 tal:content="context/summary"/>
134
135 <tal:description replace="structure context/description/fmt:text-to-html">
136 Package Description
137
138=== modified file 'lib/lp/soyuz/templates/sourcepackage-builds.pt'
139--- lib/lp/soyuz/templates/sourcepackage-builds.pt 2009-07-17 17:59:07 +0000
140+++ lib/lp/soyuz/templates/sourcepackage-builds.pt 2009-09-04 12:06:54 +0000
141@@ -3,26 +3,15 @@
142 xmlns:tal="http://xml.zope.org/namespaces/tal"
143 xmlns:metal="http://xml.zope.org/namespaces/metal"
144 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
145- xml:lang="en"
146- lang="en"
147- dir="ltr"
148- metal:use-macro="context/@@main_template/master"
149+ metal:use-macro="view/macro:page/main_only"
150 i18n:domain="launchpad"
151 >
152
153 <body>
154
155-<metal:leftportlets fill-slot="portlets_one">
156- <div tal:replace="structure context/@@+portlet-upstream" />
157-</metal:leftportlets>
158-
159-<metal:rightportlets fill-slot="portlets_two">
160- <div tal:replace="structure context/@@+portlet-releases" />
161-</metal:rightportlets>
162-
163 <div metal:fill-slot="main">
164
165- <h1><span tal:replace="context/title"/> builds</h1>
166+ <h1 tal:content="view/page_title"/>
167
168 <div tal:replace="structure context/@@+builds-list"/>
169 </div>