Code review comment for lp:~bac/launchpad/bug-432026-person-edit

Thanks for doing this Brad.

This branch looks fine to land. I have a few suggestions, but don't let them stop you from landing the code today.

> === renamed file 'lib/lp/registry/stories/foaf/xx-addemail.txt' => 'lib/lp/registry/stories/foaf/xx-add-email.txt'

I like the rename.

> --- lib/lp/registry/stories/foaf/xx-addemail.txt 2009-05-12 01:39:29 +0000
> +++ lib/lp/registry/stories/foaf/xx-add-email.txt 2009-09-17 16:18:19 +0000
> @@ -7,15 +7,13 @@
>
> Sample Person will now add a couple email addresses to his account.
>
> - # Workaround while https://launchpad.net/launchpad/+bug/39016 is not
> - # fixed.
> >>> from lp.services.mail import stub
> - >>> stub.test_emails[:] = []
> + >>> assert not stub.test_emails, (
> + ... "stub.test_emails should be empty at the start of the test.")

You can remove the assert. The problem was fixed a long time ago and lots
of tests use the stub without clearing it.

> >>> browser = setupBrowser(auth='Basic <email address hidden>:test')
> >>> browser.open('http://launchpad.dev/~name12')
> - >>> browser.getLink('Change details').click()
> - >>> browser.getLink('E-mail Settings').click()
> + >>> browser.getLink(url='+editemails').click()

This is fine as it is, but I usually use the alt text of an image to get the
link.

    >>> browser.getLink('Change e-mail settings').click()

^ Did that not work?