Code review comment for lp://staging/~gmb/launchpad/include-bnl-bug-651108

Hi Graham,

Thanks for this branch. I find it particularly interesting as I've not yet done nor reviewed an feature flag branches.

Please run the super new utilities/update-copyright script before landing. I think it only updates the files you've touched, some of which need it.

At line 144 of browser/bugsubscription.py a local variable is assigned but not used. I wonder why lint doesn't catch it? Speaking of, there are some lint issues that appear to be real.

I find the multiple tests in setUpWidgets to be confusing. Please factor out the test for is_subscribed and nest the remaining tests. It should simplify things a bit.

This comment is confusing/misleading as it implies the user can toggle the features:

183 + # When a user subscribes to a bug using the advanced features on
184 + # the Bug +subscribe page, the bug notification level they...

I think this version is more readable and accurate:

183 + # When a user subscribes to a bug with the advanced features on
184 + # the Bug +subscribe page enabled, the bug notification level they...

It is odd the current diff in the MP is showing a conflict in tales-cache.txt but none exists.