Merge lp:~abentley/launchpad/request-build into lp:launchpad/db-devel
| Status: | Merged |
|---|---|
| Approved by: | Aaron Bentley on 2010-04-09 |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | not available |
| Proposed branch: | lp:~abentley/launchpad/request-build |
| Merge into: | lp:launchpad/db-devel |
| Prerequisite: | lp:~abentley/launchpad/recipe-index |
| Diff against target: |
719 lines (+358/-105) 12 files modified
configs/development/build-from-branch.zcml (+24/-0) lib/canonical/launchpad/browser/__init__.py (+0/-4) lib/canonical/launchpad/icing/style-3-0.css.in (+11/-0) lib/lp/code/browser/sourcepackagerecipe.py (+86/-1) lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+98/-21) lib/lp/code/templates/sourcepackagerecipe-index.pt (+73/-51) lib/lp/code/templates/sourcepackagerecipe-request-builds.pt (+27/-0) lib/lp/soyuz/browser/archive.py (+14/-16) lib/lp/soyuz/browser/configure.zcml (+1/-1) lib/lp/soyuz/browser/tests/build-views.txt (+1/-1) lib/lp/testing/__init__.py (+11/-3) lib/lp/testing/factory.py (+12/-7) |
| To merge this branch: | bzr merge lp:~abentley/launchpad/request-build |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Michael Nelson (community) | ui | 2010-04-09 | Approve on 2010-04-09 |
| Paul Hummer (community) | code ui* | 2010-03-31 | Approve on 2010-04-09 |
|
Review via email:
|
|||
Description of the Change
= Summary =
Provide a UI for requesting builds, improve the display of recent/past builds
== Proposed fix ==
http://
http://
== Pre-implementation notes ==
Pre and mid-implementation discussion with rockstar.
== Implementation details ==
Tables seem to default to basically unstyled, so I created a "code" CSS class that our tables can use.
A lot of new infrastructure was needed. This includes vocabularies for the user-targettable archives and applicable distroseries. I used functions returning SimpleVocabularies rather than defining new Vocabulary classes because I couldn't see any advantage to classes, and less code is better.
As a lint fix, I added several entries to testing.__all__, because they were apparently meant to be importable from testing.
I extracted make_archive_
I also had to fix a lot of circular imports now that we're importing make_archive_
I also tweaked some factory methods and getUserBrowser, to support parameterizations I wanted to do.
== Tests ==
bin/test test_sourcepack
== Demo and Q/A ==
Cannot be Q/A'ed until more pieces are in place.
= Launchpad lint =
(As far as I can tell, this is all bogus, including the super warnings)
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files:
configs/
lib/canonical
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
== Pylint notices ==
lib/canonical/
15: [F0401] Unable to import 'lp.soyuz.
16: [F0401] Unable to import 'lp.code.
17: [F0401] Unable to import 'lp.code.
18: [F0401] Unable to import 'lp.code.
19: [F0401] Unable to import 'lp.code.
20: [F0401] Unable to import 'lp.code.
21: [F0401] Unable to import 'lp.code.
22: [F0401] Unable to import 'lp.registry.
23: [F0401] Unable to import 'lp.code.
24: [F0401] Unable to import 'lp.registry.
25: [F0401] Unable to import 'lp.soyuz.
26: [F0401] Unable to import 'lp.soyuz.
27: [F0401] Unable to import 'lp.soyuz.
28: [F0401] Unable to import 'lp.soyuz.
29: [F0401] Unable to import 'lp.soyuz.
30: [F0401] Unable to import 'lp.soyuz.
31: [F0401] Unable to import 'lp.answers.
32: [F0401] Unable to import 'lp.answers.
33: [F0401] Unable to import 'lp.answers.
34: [F0401] Unable to import 'lp.registry.
35: [F0401] Unable to import 'canonical.
36: [F0401] Unable to import 'lp.hardwaredb.
37: [F0401] Unable to import 'lp.registry.
38: [F0401] Unable to import 'canonical.
39: [F0401] Unable to import 'canonical.
40: [F0401] Unable to import 'canonical.
41: [F0401] Unable to import 'canonical.
42: [F0401] Unable to import 'lp.registry.
43: [F0401] Unable to import 'lp.registry.
44: [F0401] Unable to import 'canonical.
45: [F0401] Unable to import 'lp.registry.
46: [F0401] Unable to import 'canonical.
47: [F0401] Unable to import 'lp.registry.
48: [F0401] Unable to import 'lp.registry.
49: [F0401] Unable to import 'lp.soyuz.
50: [F0401] Unable to import 'lp.soyuz.
51: [F0401] Unable to import 'lp.answers.
52: [F0401] Unable to import 'lp.answers.
53: [F0401] Unable to import 'lp.soyuz.
54: [F0401] Unable to import 'lp.soyuz.
55: [F0401] Unable to import 'lp.blueprints.
56: [F0401] Unable to import 'lp.blueprints.
57: [F0401] Unable to import 'lp.blueprints.
58: [F0401] Unable to import 'lp.blueprints.
59: [F0401] Unable to import 'lp.blueprints.
60: [F0401] Unable to import 'lp.blueprints.
61: [F0401] Unable to import 'lp.blueprints.
62: [F0401] Unable to import 'lp.blueprints.
63: [F0401] Unable to import 'lp.blueprints.
64: [F0401] Unable to import 'lp.registry.
65: [F0401] Unable to import 'lp.registry.
66: [F0401] Unable to import 'canonical.
67: [F0401] Unable to import 'canonical.
lib/lp/
11: [F0401] Unable to import 'zope.component'
13: [F0401] Unable to import 'zope.schema'
14: [F0401] Unable to import 'zope.schema.
15: [F0401] Unable to import 'canonical.
17: [F0401] Unable to import 'canonical.
18: [F0401] Unable to import 'canonical.
21: [F0401] Unable to import 'canonical.
22: [F0401] Unable to import 'lp.buildmaster
23: [F0401] Unable to import 'lp.code.
24: [F0401] Unable to import 'lp.soyuz.
25: [F0401] Unable to import 'lp.soyuz.
27: [F0401] Unable to import 'lp.registry.
28: [F0401] Unable to import 'lp.registry.
lib/lp/
29: [E1002, TestSourcePacka
lib/lp/
31: [F0401] Unable to import 'pytz'
34: [F0401] Unable to import 'zope.app.
35: [F0401] Unable to import 'zope.component'
36: [F0401] Unable to import 'zope.formlib'
38: [F0401] Unable to import 'zope.security.
39: [F0401] Unable to import 'zope.schema'
40: [F0401] Unable to import 'zope.schema.
41: [F0401] Unable to import 'zope.schema.
42: [F0401] Unable to import 'storm.
46: [F0401] Unable to import 'canonical.
47: [F0401] Unable to import 'canonical.
48: [F0401] Unable to import 'canonical.
49: [F0401] Unable to import 'canonical.
50: [F0401] Unable to import 'lp.buildmaster
51: [F0401] Unable to import 'lp.services.
52: [F0401] Unable to import 'lp.services.
53: [F0401] Unable to import 'lp.soyuz.
54: [F0401] Unable to import 'lp.soyuz.
56: [F0401] Unable to import 'canonical.
57: [F0401] Unable to import 'lp.soyuz.
59: [F0401] Unable to import 'lp.soyuz.
61: [F0401] Unable to import 'lp.soyuz.
64: [F0401] Unable to import 'lp.soyuz.
66: [F0401] Unable to import 'lp.soyuz.
67: [F0401] Unable to import 'lp.soyuz.
68: [F0401] Unable to import 'lp.soyuz.
69: [F0401] Unable to import 'lp.soyuz.
70: [F0401] Unable to import 'lp.soyuz.
71: [F0401] Unable to import 'lp.registry.
72: [F0401] Unable to import 'canonical.
74: [F0401] Unable to import 'lp.soyuz.
76: [F0401] Unable to import 'lp.soyuz.
77: [F0401] Unable to import 'lp.registry.
78: [F0401] Unable to import 'lp.registry.
79: [F0401] Unable to import 'lp.soyuz.
82: [F0401] Unable to import 'lp.registry.
84: [F0401] Unable to import 'canonical.
88: [F0401] Unable to import 'lp.soyuz.
89: [F0401] Unable to import 'canonical.
90: [F0401] Unable to import 'canonical.
91: [F0401] Unable to import 'canonical.
92: [F0401] Unable to import 'canonical.
93: [F0401] Unable to import 'canonical.
94: [F0401] Unable to import 'canonical.
95: [F0401] Unable to import 'canonical.widgets'
97: [F0401] Unable to import 'canonical.
99: [F0401] Unable to import 'canonical.
101: [F0401] Unable to import 'canonical.
791: [E1002, ArchiveView.
989: [E1002, ArchiveSourceSe
lib/lp/
57: [F0401] Unable to import 'pytz'
58: [F0401] Unable to import 'storm.expr'
59: [F0401] Unable to import 'storm.store'
60: [F0401] Unable to import 'storm.tracer'
63: [F0401] Unable to import 'transaction'
67: [F0401] Unable to import 'windmill.
69: [F0401] Unable to import 'zope.component'
70: [F0401] Unable to import 'zope.event'
72: [F0401] Unable to import 'zope.security.
74: [F0401] Unable to import 'zope.testing.
76: [F0401] Unable to import 'canonical.
77: [F0401] Unable to import 'canonical.config'
78: [F0401] Unable to import 'canonical.
79: [F0401] Unable to import 'canonical.
80: [F0401] Unable to import 'lp.codehosting
83: [F0401] Unable to import 'lp.testing._login'
87: [F0401] Unable to import 'lp.testing._tales'
88: [F0401] Unable to import 'lp.testing.
201: [F0401, run_with_
375: [F0401, TestCase.setUp] Unable to import 'lp.testing.
404: [F0401, TestCaseWithFac
416: [F0401, TestCaseWithFac
542: [F0401, TestCaseWithFac
606: [F0401, YUIUnitTestCase
lib/lp/
27: [F0401] Unable to import 'pytz'
28: [F0401] Unable to import 'storm.store'
29: [F0401] Unable to import 'transaction'
33: [F0401] Unable to import 'zope.component'
34: [F0401] Unable to import 'zope.security.
36: [F0401] Unable to import 'canonical.
37: [F0401] Unable to import 'canonical.config'
38: [F0401] Unable to import 'canonical.
41: [F0401] Unable to import 'canonical.
42: [F0401] Unable to import 'canonical.
43: [F0401] Unable to import 'canonical.
44: [F0401] Unable to import 'canonical.
45: [F0401] Unable to import 'canonical.
47: [F0401] Unable to import 'canonical.
49: [F0401] Unable to import 'canonical.
50: [F0401] Unable to import 'lp.hardwaredb.
53: [F0401] Unable to import 'canonical.
54: [F0401] Unable to import 'canonical.
55: [F0401] Unable to import 'canonical.
58: [F0401] Unable to import 'canonical.
59: [F0401] Unable to import 'canonical.
62: [F0401] Unable to import 'lp.blueprints.
64: [F0401] Unable to import 'lp.blueprints.
66: [F0401] Unable to import 'lp.bugs.
67: [F0401] Unable to import 'lp.bugs.
68: [F0401] Unable to import 'lp.bugs.
69: [F0401] Unable to import 'lp.bugs.
70: [F0401] Unable to import 'lp.buildmaster
71: [F0401] Unable to import 'lp.buildmaster
72: [F0401] Unable to import 'lp.buildmaster
74: [F0401] Unable to import 'lp.code.enums'
79: [F0401] Unable to import 'lp.code.errors'
80: [F0401] Unable to import 'lp.code.
81: [F0401] Unable to import 'lp.code.
82: [F0401] Unable to import 'lp.code.
83: [F0401] Unable to import 'lp.code.
84: [F0401] Unable to import 'lp.code.
85: [F0401] Unable to import 'lp.code.
86: [F0401] Unable to import 'lp.code.
87: [F0401] Unable to import 'lp.code.
88: [F0401] Unable to import 'lp.code.
89: [F0401] Unable to import 'lp.code.
92: [F0401] Unable to import 'lp.code.
93: [F0401] Unable to import 'lp.codehosting
95: [F0401] Unable to import 'lp.registry.
96: [F0401] Unable to import 'lp.registry.
98: [F0401] Unable to import 'lp.registry.
99: [F0401] Unable to import 'lp.registry.
100: [F0401] Unable to import 'lp.registry.
102: [F0401] Unable to import 'lp.registry.
104: [F0401] Unable to import 'lp.registry.
106: [F0401] Unable to import 'lp.registry.
107: [F0401] Unable to import 'lp.registry.
108: [F0401] Unable to import 'lp.registry.
109: [F0401] Unable to import 'lp.registry.
110: [F0401] Unable to import 'lp.registry.
111: [F0401] Unable to import 'lp.registry.
113: [F0401] Unable to import 'lp.registry.
115: [F0401] Unable to import 'lp.registry.
116: [F0401] Unable to import 'lp.registry.
118: [F0401] Unable to import 'lp.registry.
119: [F0401] Unable to import 'lp.registry.
120: [F0401] Unable to import 'lp.registry.
122: [F0401] Unable to import 'lp.services.
123: [F0401] Unable to import 'lp.services.
124: [F0401] Unable to import 'lp.services.
126: [F0401] Unable to import 'lp.soyuz.
128: [F0401] Unable to import 'lp.soyuz.
129: [F0401] Unable to import 'lp.soyuz.
130: [F0401] Unable to import 'lp.soyuz.
131: [F0401] Unable to import 'lp.soyuz.
132: [F0401] Unable to import 'lp.soyuz.
133: [F0401] Unable to import 'lp.soyuz.
134: [F0401] Unable to import 'lp.soyuz.
135: [F0401] Unable to import 'lp.soyuz.
137: [F0401] Unable to import 'lp.testing'
139: [F0401] Unable to import 'lp.translation
140: [F0401] Unable to import 'lp.translation
142: [F0401] Unable to import 'lp.translation
143: [F0401] Unable to import 'lp.translation
144: [F0401] Unable to import 'lp.translation
1756: [F0401, LaunchpadObject
| Aaron Bentley (abentley) wrote : | # |
| Paul Hummer (rockstar) wrote : | # |
Here are the things we discussed on the phone today that should be improvements:
- The request-build page should only show "active" distroseries.
- The recipe index page should have the build history in a table, so that the data is easier to scan (line up the various parts of the history)
| Aaron Bentley (abentley) wrote : | # |
Here are updated screenshots:
http://
http://
| Paul Hummer (rockstar) wrote : | # |
Based on the screenshot at:
http://
This looks good. Moving the table down to be a full width and making description half width seems to be the best plan. Thanks for adding the padding below the table for "Request a Build." I think this UI is complete.
| Paul Hummer (rockstar) wrote : | # |
70 +table.code {
71 + margin-bottom: 1em;
72 +}
73 +table.code th {
74 + text-align: left;
75 + font-weight: bold;
76 +}
77 +table.code th, table.code td {
78 + padding-right: 3em;
79 +}
80 +
I'd rather see this implemented as a specific id, since it's still pretty specific to the column count (for padding) and where the table is used.
157 + class schema(Interface):
158 + """Schema for requesting a build."""
159 + distros = List(
160 + Choice(
161 + title=u'
162 + archive = Choice(
163 +
I like this. I think it makes for less clutter. Have we done this before somewhere? I'm worried it might be inconsistent, even though it's better.
Other than that, this looks good. Thanks for working on it.
| Michael Nelson (michael.nelson) wrote : | # |
Hi Aaron and Paul,
Looking great.
Regarding the table styling, did you try adding the 'listing' class? That's what is used in other places (see table.listing in both style.css and style-3-0.css.in) for styling tables that list items. It would be great to re-use it for consistency rather than creating another table listing style if possible.
I think I forgot to mention during the review for the recipe index page, but I had a question about the breadcrumbs... shouldn't they be:
Person-name5 >> Branches >> generic_string23 recipe and
Person-name5 >> Branches >> generic_string23 recipe >> Request builds
?
And lastly, I was surprised to see non-ubuntu distroseries there for selection? Currently we only support PPA's where the distribution is ubuntu? (see lp.soyuz.
| Aaron Bentley (abentley) wrote : | # |
On 04/09/2010 06:30 AM, Michael Nelson wrote:
> Review: Needs Information ui
> Hi Aaron and Paul,
>
> Looking great.
>
> Regarding the table styling, did you try adding the 'listing' class?
No. Thanks for pointing it out.
However, it does not work very well:
http://
The table is unnecessarily wide, and the headings are not aligned with
the cell contents.
> I think I forgot to mention during the review for the recipe index page, but I had a question about the breadcrumbs... shouldn't they be:
>
> Person-name5>> Branches>> generic_string23 recipe and
> Person-name5>> Branches>> generic_string23 recipe>> Request builds
> ?
>
> And lastly, I was surprised to see non-ubuntu distroseries there for selection?
We're aware that we may need to tweak this before going live, but it has
been difficult to get answers about exactly what selection criteria
should be used, so we went with simply listing active distroseries.
Aaron
| Aaron Bentley (abentley) wrote : | # |
On 04/09/2010 12:37 AM, Paul Hummer wrote:
> Review: Approve code ui*
> 70 +table.code {
> 71 + margin-bottom: 1em;
> 72 +}
> 73 +table.code th {
> 74 + text-align: left;
> 75 + font-weight: bold;
> 76 +}
> 77 +table.code th, table.code td {
> 78 + padding-right: 3em;
> 79 +}
> 80 +
>
> I'd rather see this implemented as a specific id, since it's still pretty specific to the column count (for padding) and where the table is used.
How is it specific to the column count, or where the table is used?
I really want tables to not be broken, and to me that means:
- left-aligned headings
- bold (or otherwise emphasized) headings
- reasonable padding between columns
- blank space between the bottom of the table and the next element
If it wasn't disruptive, I would make this the default style for all tables.
> 157 + class schema(Interface):
> 158 + """Schema for requesting a build."""
> 159 + distros = List(
> 160 + Choice(
> 161 + title=u'
> 162 + archive = Choice(
> 163 +
>
> I like this. I think it makes for less clutter.
The fact that it's an inner class named schema, instead of being defined
externally and then assigned as a member?
> Have we done this before somewhere? I'm worried it might be inconsistent, even though it's better.
A quick grep suggests this is the first time we've done this. I hope
not the last.
Aaron
| Michael Nelson (michael.nelson) wrote : | # |
On Fri, Apr 9, 2010 at 4:15 PM, Aaron Bentley <email address hidden> wrote:
> On 04/09/2010 06:30 AM, Michael Nelson wrote:
>> Review: Needs Information ui
>> Hi Aaron and Paul,
>>
>> Looking great.
>>
>> Regarding the table styling, did you try adding the 'listing' class?
>
> No. Thanks for pointing it out.
>
> However, it does not work very well:
> http://
>
> The table is unnecessarily wide,
There is .narrow-listing too (which you would need to add in addition
to .listing), if that helps (as yes, the .listing class defaults to
100%)
> and the headings are not aligned with
> the cell contents.
I *think* this is because all the listing styles assume that your
table uses the thead/tbody elements, which you're not afaics. See:
lib/canonical/
>
>> I think I forgot to mention during the review for the recipe index page, but I had a question about the breadcrumbs... shouldn't they be:
>>
>> Person-name5>> Branches>> generic_string23 recipe and
>> Person-name5>> Branches>> generic_string23 recipe>> Request builds
>> ?
Any comment about this?
>>
>> And lastly, I was surprised to see non-ubuntu distroseries there for selection?
>
> We're aware that we may need to tweak this before going live, but it has
> been difficult to get answers about exactly what selection criteria
> should be used, so we went with simply listing active distroseries.
I see. My assumption is that it's simply active ubuntu distroseries.
Hope that helps!
>
> Aaron
> --
> https:/
> You are reviewing the proposed merge of lp:~abentley/launchpad/request-build into lp:launchpad.
>
| Michael Nelson (michael.nelson) wrote : | # |
Forgot to mark this as approved, assuming you think about the previous comment :)
| Aaron Bentley (abentley) wrote : | # |
On 04/09/2010 11:18 AM, Michael Nelson wrote:
> On Fri, Apr 9, 2010 at 4:15 PM, Aaron Bentley<email address hidden> wrote:
>> The table is unnecessarily wide,
>
> There is .narrow-listing too (which you would need to add in addition
> to .listing), if that helps (as yes, the .listing class defaults to
> 100%)
That is much better.
> I *think* this is because all the listing styles assume that your
> table uses the thead/tbody elements, which you're not afaics. See:
>
> lib/canonical/
I don't understand why the style would assume that. It would be much
nicer to only expect thead/tbody when that provides functionality.
Anyhow, it's much improved:
http://
I'd rather have left-aligned headings, but anything's better than
right-aligned.
>>> I think I forgot to mention during the review for the recipe index page, but I had a question about the breadcrumbs... shouldn't they be:
>>>
>>> Person-name5>> Branches>> generic_string23 recipe and
>>> Person-name5>> Branches>> generic_string23 recipe>> Request builds
>>> ?
>
> Any comment about this?
I'd rather be eating toast than thinking about breadcrumbs?
Aaron

Here are some screenshots: people. canonical. com/~abentley/ recipe- index-detailed. png people. canonical. com/~abentley/ request- build.png
http://
http://