Code review comment for lp:~jcsackett/launchpad/unsubscription

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Jon.

This looks good to land after you have addressed my nitpick about the docstring and id.

> === added file 'lib/lp/registry/browser/tests/test_mailinglists.py'
> --- lib/lp/registry/browser/tests/test_mailinglists.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/registry/browser/tests/test_mailinglists.py 2010-08-17 16:07:46 +0000
...

+class MailingListSubscriptionControlsTestCase(TestCaseWithFactory):
+ """Tests to ensure the rendering of subscribe and unsubscribe
+ controls on the team page."""

PEP 257 states that a docstring has a single line synopses.
    """Verify the team index subscribe/unsubscribe to mailing list content."""

...

> === modified file 'lib/lp/registry/templates/team-portlet-mailinglist.pt'
> --- lib/lp/registry/templates/team-portlet-mailinglist.pt 2010-06-04 15:37:24 +0000
> +++ lib/lp/registry/templates/team-portlet-mailinglist.pt 2010-08-17 16:07:46 +0000
> @@ -19,10 +19,10 @@
> <strong>Policy:</strong>
> You must be a team member to subscribe to the team mailing list.
> <br/>
> - <tal:member condition="view/user_is_active_member">
> + <tal:member condition="view/userIsParticipant">
> <tal:can-subscribe-to-list
> condition="view/user_can_subscribe_to_list">
> - <a class="sprite add"
> + <a id="link.list.subscribe" class="sprite add"

This is not a valid CSS3 id use dashes, no dots, no underscores. We know that
zope and some of out code are making bad ids. We need to get the site ready
for the CSS3 standard.

review: Approve (code)

« Back to merge proposal