Code review comment for lp:~wallyworld/launchpad/blueprint-subscription-forms

Revision history for this message
Benji York (benji) wrote :

This branch looks good. I had a couple of thoughts while reading over
it, but nothing that would keep us from landing it.

In specificationsubscription.py (line 221 of the diff), would it be
better to say "Modify subscription to %s" (adding the "to")?

I also noticed that the elements of __all__ in
specificationsubscription.py aren't quite in alphabetical order.

This test confuses me:

Or we can click the icon next to their name to get to the subscription edit
page.

    >>> browser.getLink(url='/+subscription/stub').click()
    >>> browser.getControl(name='field.essential').value = 'no'
    >>> browser.getControl('Change').click()

I don't see any assertions. Maybe instead of the second and third lines
there should be a check to be sure the current URL is what is expected.

review: Approve (code)

« Back to merge proposal