Code review comment for lp://staging/~tealeg/landscape-client/use-configobj

Looks good! +1

[1]

+from configobj import ConfigObj

We need to add a Depends entry for this in debian/control (and check that it's available and on all supported releases).

[2]

+ def get_config_object(self, config_source=None):

Please add a docstring and make this private, since it's not meant to be used by calling code.

[3]

+ if config_source:
+ config_obj = ConfigObj(config_source)
+ else:
+ config_obj = ConfigObj(self.get_config_filename())

Please prefer removing code duplication when instantiating objects, e.g.:

filename = config_source or self.get_config_filename()
config_obj = ConfigObj(filename)

or:

if config_source is not None:
    filename = config_source
else:
    filename = self.get_config_filename()
config_obj = ConfigObj(filename)