Code review comment for lp:~edwin-grubbs/launchpad/bug-430610-sprint-settopics.pt-3.0-layout

On Sep 18, 2009, at 06:10 PM, Edwin Grubbs wrote:

>Edwin Grubbs has proposed merging lp:~edwin-grubbs/launchpad/bug-430610-sprint-settopics.pt-3.0-layout into lp:launchpad/devel.
>
>Requested reviews:
> Canonical Launchpad Engineering (launchpad): code
>
>Converted sprint-settopics.pt to UI 3.0.
>
>./bin/test -vvt sprint

Thanks for another great blueprint template update Edwin. I have some minor
comments that should be easy to resolve. merge-conditional, r=me with their
consideration.

 review approve code ui*
 status approve

=== modified file 'lib/lp/blueprints/browser/sprint.py'
--- lib/lp/blueprints/browser/sprint.py 2009-09-03 15:04:13 +0000
+++ lib/lp/blueprints/browser/sprint.py 2009-09-18 18:06:53 +0000
> @@ -50,6 +50,7 @@
> from canonical.launchpad.webapp.breadcrumb import Breadcrumb
> from canonical.launchpad.helpers import shortlist
> from canonical.widgets.date import DateTimeWidget
> +from canonical.lazr.utils import smartquote

Please sort these imports.

> class SprintFacets(StandardLaunchpadFacets):
> @@ -327,7 +328,15 @@
>
> It is unusual because we want to display multiple objects with
> checkboxes, then process the selected items, which is not the usual
> - add/edit metaphor."""
> + add/edit metaphor.
> + """
> +
> + @property
> + def label(self):
> + return smartquote(
> + 'Review discussion topics for "%s" sprint' % self.context.title)
> +
> + page_title = label

I bet you don't need the page_title. You should remove this so the default
reverse-breadcrumbs are used.

=== modified file 'lib/lp/blueprints/stories/sprints/sprint-settopics.txt'
--- lib/lp/blueprints/stories/sprints/sprint-settopics.txt 2009-08-13 19:03:36 +0000
+++ lib/lp/blueprints/stories/sprints/sprint-settopics.txt 2009-09-16 10:45:22 +0000
> @@ -70,7 +70,8 @@
> >>> cprov_browser.getLink('Set agenda').click()
>
> >>> cprov_browser.title
> - 'Review topics proposed for discussion at Ubuntu DevSummit Guacamole'
> + '+settopics : Blueprints for Ubuntu DevSummit Guacamole : Ubuntu
> + DevSummit Guacamole : Meetings'

You should print the title so the quotes aren't shown.