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

[1] pyflakes

landscape/ui/controller/tests/test_app.py:14: redefinition of unused 'SettingsApplicationController' from line 9
landscape/ui/controller/tests/test_configuration.py:6: 'ConfigControllerLockError' imported but unused
landscape/ui/controller/tests/test_configuration.py:12: redefinition of unused 'ConfigController' from line 6
landscape/ui/model/configuration/tests/test_state.py:12: redefinition of unused 'landscape' from line 6
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'ConfigurationModel' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'StateError' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'VirginState' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'InitialisedState' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'ModifiedState' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'HOSTED' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'LOCAL' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'HOSTED_LANDSCAPE_HOST' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'LANDSCAPE_HOST' from line 7
landscape/ui/model/configuration/tests/test_state.py:13: redefinition of unused 'COMPUTER_TITLE' from line 7

[2]
+ try:
+ setattr(self._configuration, name, value)
+ self._configuration.modify()
+ except AttributeError:
+ return object.__setattr__(self, name, value)

I think the modify call should be in the else clause.

[3] Can you use triple quotes for docstring everywhere, in particular ConfigController?

[4]
+ self.default_data = {"management-type": "not",
+ "computer-title": "",
+ "hosted-landscape-host": "",
+ "hosted-account-name": "",
+ "hosted-password": "",
+ "local-landscape-host": "",
+ "local-account-name": "",
+ "local-password": ""
+ }

Nitpicking again, but the last "}" should be on the line before.

[5]
+from landscape.ui.constants import (
+ CANONICAL_MANAGED, NOT_MANAGED)

This can be on one line.

[6] It'd be nice to have some more docstrings on ClientSettingsDialog.

Great work, +1!