Code review comment for lp://staging/~tealeg/landscape-client/ui-interface-iteration3

Revision history for this message
Fernando Correa Neto (fcorrea) wrote :

Looks great!

[1]

427 + except StateError:
428 + # We probably don't care.
429 + pass

Even if we don't care, maybe we should log something instead of not handling it?

[2]

975 + self._state._proxy.account_name = \
976 + self._state.get(first_key, ACCOUNT_NAME)
977 + self._state._proxy.registration_password = \
978 + self._state.get(first_key, PASSWORD)

I've noticed in previous reviews that we should avoid breaking likes using backslash. Maybe the following could make it consistent with the rest of our codebase.

            self._state._proxy.account_name = self._state.get(
                first_key, ACCOUNT_NAME)
            self._state._proxy.registration_password = self._state.get(
                first_key, PASSWORD)

(Didn't check if that goes beyond the 79char limit)

Also, do you think it would make sense to move l.ui.tests.helpers to a l.ui.testing module so it's more consistent with what we have in the server?
No biggie on this one, just trying to keep consistency at all costs :)

+1!

review: Approve

« Back to merge proposal