Code review comment for lp:~gary/launchpad/launchpad-templates-2

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Gary,

This is a nice branch. Just a few changes.

merge-conditional

-Edwin

>=== modified file 'lib/canonical/launchpad/browser/temporaryblobstorage.py'
>--- lib/canonical/launchpad/browser/temporaryblobstorage.py 2009-06-25 05:30:52 +0000
>+++ lib/canonical/launchpad/browser/temporaryblobstorage.py 2009-09-17 22:21:52 +0000
>@@ -20,6 +20,7 @@
> class TemporaryBlobStorageAddView(LaunchpadFormView):

Can you add an XXX referencing bug 315358 and a comment that this page
could possibly be removed after that bug has been completed.

> schema = ITemporaryBlobStorage
> label = 'Store BLOB'
>+ page_title = 'Store a BLOB temporarily in Launchpad'
> field_names = ['blob']
> for_input = True
>
>
>=== modified file 'lib/canonical/launchpad/templates/root-index.pt'
>--- lib/canonical/launchpad/templates/root-index.pt 2009-08-03 10:00:43 +0000
>+++ lib/canonical/launchpad/templates/root-index.pt 2009-09-17 22:21:52 +0000
>@@ -3,10 +3,7 @@
> xmlns:tal="http://xml.zope.org/namespaces/tal"
> xmlns:metal="http://xml.zope.org/namespaces/metal"
> xmlns:i18n="http://xml.zope.org/namespaces/i18n"
>- xml:lang="en"
>- lang="en"
>- dir="ltr"
>- metal:use-macro="view/macro:page/search"
>+ metal:use-macro="view/macro:page/searchless"

You didn't remove the root_index variable from pagetitles.py. BTW, this
seems like a fourth template, so I will have to charge extra.

> i18n:domain="launchpad">
> <body>
> <div metal:fill-slot="main">
>
>=== modified file 'lib/canonical/launchpad/zcml/launchpad.zcml'
>--- lib/canonical/launchpad/zcml/launchpad.zcml 2009-09-04 15:32:52 +0000
>+++ lib/canonical/launchpad/zcml/launchpad.zcml 2009-09-17 22:21:52 +0000
>@@ -256,6 +256,7 @@
> <browser:page
> for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
> name="+graphics"
>+ class="canonical.launchpad.browser.LaunchpadGraphics"

Should specify exact module, e.g.
"canonical.launchpad.browser.launchpad.LaunchpadGraphics".

> template="../templates/launchpad-graphics.pt"
> permission="zope.Public"
> />
>
>=== modified file 'lib/canonical/launchpad/zcml/launchpadstatistic.zcml'
>--- lib/canonical/launchpad/zcml/launchpadstatistic.zcml 2009-07-13 18:15:02 +0000
>+++ lib/canonical/launchpad/zcml/launchpadstatistic.zcml 2009-09-17 22:21:52 +0000
>@@ -35,6 +35,7 @@
> <browser:pages
> for="canonical.launchpad.interfaces.ILaunchpadStatisticSet"
> facet="overview"
>+ class="canonical.launchpad.browser.LaunchpadStatisticSet"

Here, too.

> permission="launchpad.Admin">
> <browser:page
> name="+index"
>

review: Approve (code)

« Back to merge proposal