Code review comment for lp:~nataliabidart/ubuntuone-client/login-email-password-for-everyone

Revision history for this message
Alejandro J. Cura (alecu) wrote :

Very nice cleanup branch.

Some issues:

----

There's a latent issue in making the variable "done" an instance variable in CredentialsManagementTool, and using it through all the inlineCallbacks generators in this class.

The "done" should be different for each of this generators, since more than one can be used at once.

I strongly believe instance variables *are not* clean to use here, and that they should be left as variables local to the generator. And perhaps we can have callback_done and errback_done get the "done" deferred from each generator via functools.partial.

----

This docstring is wrong (looks duplicated from the one above):

    def CredentialsStored(self):
        """Signal thrown when the credentials were cleared."""

----

The empty line should be restored before class CredentialsError.

----

review: Needs Fixing (just eyeballed the code)

« Back to merge proposal