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

Revision history for this message
Ian Booth (wallyworld) wrote :

Hi Benji

Thanks for the review.

>
> 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.
>

Will fix.

> 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.
>

There was an existing doc test that simply did the above that I reused.
Yes, it should have an assert (as should the existing test). I'll add
one in.

« Back to merge proposal