Code review comment for lp:~ursinha/launchpad/add-translation-focus

Revision history for this message
Brad Crittenden (bac) wrote :

Hi Ursula,

Thanks for this patch.

As we discussed on IRC, the tests beginning at line 53 need to have the "if product_series" factored out so it isn't repeated and the other simpler tests nested. Easy fix that will enhance the readability.

Also this change will need some tests. Likely you'll need a doc test, a story, and a web services test to show the value can be changed. As you know we don't have automated tests for access via launchpadlib but doing an interactive session to get and set it will give us confidence it works.

review: Needs Fixing (code)

« Back to merge proposal