Code review comment for lp:~nataliabidart/ubuntu-sso-client/unify-signal-broadcaster

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

> Why do we have an Elf here?

Fixed.

> I think __init__ should be deleted, and each of those lambda methods could be
> written as:
> def LoggedIn(self, app_name, result):
> d.callback(result)

Fixed.

> "Provides an utility" -> "Provides a utility"

Fixed.

> I find that the refcounting code in CredentialsManagement is confusing.
> Perhaps we should open a bug to fix this so it's not spread in so many
> functions, and also we should make so that when the refcount increases above
> zero any pending timeout should be cancelled.

I agree. Filled bug #914381.

« Back to merge proposal