Launchpad itself

Merge lp:~wgrant/launchpad/bug-540819-fix-builder-list-icons into lp:launchpad

  1. bug-540819-fix-builder-list-icons
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~wgrant/launchpad/bug-540819-fix-builder-list-icons
Merge into: lp:launchpad
Diff against target: 64 lines (+3/-11)
4 files modified
lib/canonical/launchpad/webapp/tales.py (+2/-9)
lib/lp/soyuz/templates/builder-index.pt (+1/-0)
lib/lp/soyuz/templates/buildfarmbranchjob-current.pt (+0/-1)
lib/lp/soyuz/templates/buildfarmjob-current.pt (+0/-1)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-540819-fix-builder-list-icons
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+22288@code.launchpad.net

Commit message

Move the icon for package builds in builder listings back to the start of the text, where it is for the other build types.

Description of the change

This branch fixes the icon positioning on https://edge.launchpad.net/builders to match production's. It was broken a week or so ago during buildfarm generalisation changes.

Before: http://people.ubuntu.com/~wgrant/launchpad/bug-540819/before.png
After: http://people.ubuntu.com/~wgrant/launchpad/bug-540819/after.png

This brings the binary build rendering back into line with historical behaviour, except that it will always show the BUILDING icon. Previously it would occasionally show the SFULLYBUILT icon for a fraction of a second at the end of a build, which could probably have been called a bug anyway.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

Has this had a preimplementation call? Is there no fmt:link formatter for these items, or is it somehow inappropriate?

Revision history for this message
William Grant (wgrant) wrote :

I discussed this a week or so ago with jtv and noodles, IIRC. Note that for Builds and SPRBs we already use fmt:link here -- but including the icon and other text in it is only appropriate in this particular context.

Revision history for this message
William Grant (wgrant) wrote :

20:55 < wgrant> jtv: The 'building' icon spacing on https://edge.launchpad.net/builders seems a bit broken. Is that related to your changes?
20:56 < jtv> wgrant: yes, it is. How appropriate it is depends a bit on your perspective: it's the build status, so I'm displaying it as part of the link to the build. It did simplify the code.
20:57 < jtv> wgrant: in fact this is the subject matter of a UI review that's currently somewhere in limbo.
20:57 < wgrant> jtv: The current spacing is definitely broken. I think the ordering is too, but that's up for debate.
20:58 < jtv> wgrant: I don't think I changed the ordering... did I?
20:58 < wgrant> It was '( ) Building _blah blah blah i386_'
20:58 < wgrant> It is now 'Building ( )_blah blah blah i386_'
20:59 < jtv> wgrant: oh, sorry, misunderstood you earlier. That's the part I was talking about all along.
20:59 < wgrant> jtv: Why does it make the code simpler?
20:59 < wgrant> Because it was looking at the build status to generate the icon?
21:00 < jtv> wgrant: and because it was easier to have a link formatter for builds.
21:01 < wgrant> jtv: Argh, where's the template for that?
21:01 * wgrant cannot find it.
21:01 < jtv> builder-index.pt
21:03 < wgrant> jtv: Looks like buildfarmbuildjob-current.pt
21:03 < jtv> ah yes, that's where the link happens now.
21:03 < jtv> That was another thing: we can't show that icon for a buildless job.
21:04 < jtv> So that's another way in which it belongs to the build, not the builder.
21:04 < wgrant> So, I think we should do what is done for the rest of the states.
21:04 < wgrant> And just use the currentlybuilding icon if there's a job assigned.
21:04 < wgrant> Regardless of state.
21:04 < jtv> That's fine by me.
21:06 < wgrant> jtv: Ah, so you already hardcode that icon form non-build BFJs?
21:07 < jtv> wgrant: I _think_ I did that in an earlier iteration but it didn't seem appropriate.
21:07 < wgrant> It appears to be in devel's buildfarmjob-current.pt
21:08 < jtv> wgrant: there was a lot of exploring to get it all in a sort of minimal working shape... I don't mind moving things around a bit now that we have a working situation.
21:09 < wgrant> Yep.

Revision history for this message
Aaron Bentley (abentley) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/tales.py'
2--- lib/canonical/launchpad/webapp/tales.py 2010-03-20 22:11:01 +0000
3+++ lib/canonical/launchpad/webapp/tales.py 2010-03-30 08:05:49 +0000
4@@ -1523,22 +1523,15 @@
5 else:
6 return ""
7
8- def icon(self, view_name):
9- if not check_permission('launchpad.View', self._context):
10- return '<img src="/@@/processing" alt="[build]" />'
11-
12- return BuildImageDisplayAPI(self._context).icon()
13-
14 def link(self, view_name, rootsite=None):
15- icon = self.icon(view_name)
16 build = self._context
17 if not check_permission('launchpad.View', build):
18- return '%s private source' % icon
19+ return 'private source'
20
21 url = self.url(view_name=view_name, rootsite=rootsite)
22 title = cgi.escape(build.title)
23 archive = self._composeArchiveReference(build.archive)
24- return '<a href="%s">%s%s</a>%s' % (url, icon, title, archive)
25+ return '<a href="%s">%s</a>%s' % (url, title, archive)
26
27
28 class CodeImportMachineFormatterAPI(CustomizableFormatter):
29
30=== modified file 'lib/lp/soyuz/templates/builder-index.pt'
31--- lib/lp/soyuz/templates/builder-index.pt 2010-03-16 11:07:39 +0000
32+++ lib/lp/soyuz/templates/builder-index.pt 2010-03-30 08:05:49 +0000
33@@ -106,6 +106,7 @@
34
35 <tal:job-header condition="job">
36 <span class="sortkey" tal:content="job/id" />
37+ <img src="/@@/processing" alt="[building]" />
38 <tal:specific-job replace="structure job/specific_job/@@+current" />
39 </tal:job-header>
40
41
42=== modified file 'lib/lp/soyuz/templates/buildfarmbranchjob-current.pt'
43--- lib/lp/soyuz/templates/buildfarmbranchjob-current.pt 2010-03-16 05:08:47 +0000
44+++ lib/lp/soyuz/templates/buildfarmbranchjob-current.pt 2010-03-30 08:05:49 +0000
45@@ -4,7 +4,6 @@
46 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
47 omit-tag="">
48
49- <img src="/@@/processing" alt="[building]" />
50 Working on
51 <tal:jobtype replace="context/__class__/__name__" />
52 for branch <tal:branch replace="structure context/branch/fmt:link" />.
53
54=== modified file 'lib/lp/soyuz/templates/buildfarmjob-current.pt'
55--- lib/lp/soyuz/templates/buildfarmjob-current.pt 2010-03-16 05:08:47 +0000
56+++ lib/lp/soyuz/templates/buildfarmjob-current.pt 2010-03-30 08:05:49 +0000
57@@ -4,7 +4,6 @@
58 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
59 omit-tag="">
60
61- <img src="/@@/processing" alt="[building]" />
62 Working on
63 <tal:jobtype replace="context/__class__/__name__" />.
64 </tal:root>