launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08060
Re: [Merge] lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad
Review: Approve code
This branch looks good.
I have a couple of small page template suggestions:
The <div>s that is used to tal:define the "classname" string will show
up in the generated output, which won't kill kittens or anything, but it
can be avoided by using a tal-namespaced element instead, like so:
<tal:conditional condition="not: container/has_incomplete_work">
<tal:classname define="global classname string:"/>
</tal:conditional>
Note that since the element has the "tal" namespace you don't have to
put it on the "define" attribute.
The other is that since "classname" is global (which is probably the
right choice here) a name less likely to cause name collisions is
appropriate. Say "upcoming-work-class-name", or something like that.
Thanks for the branch; this will really improve the work items list.
--
https://code.launchpad.net/~dooferlad/launchpad/upcomingwork_show_incomplete_bp/+merge/106438
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References