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

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

Why do we have an Elf here?

            def __init__(elf):
                elf.LoggedIn = lambda app_name, result: d.callback(result)
                elf.LoginError = lambda app_name, error: d.errback(error)
                elf.UserNotValidated = lambda app_name, email: d.callback(None)

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)
---

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

---

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 don't consider this to be a "needs-fixing" since it's code that was already present in this branch.

« Back to merge proposal