Code review comment for lp:~ian-clatworthy/bzr/411413-plugin-disable

Revision history for this message
John A Meinel (jameinel) wrote :

161 + try:
162 + enabled = load_rules[f]
163 + if not enabled:
164 + trace.mutter('Plugin name %s disabled', f)
165 + continue
166 + except KeyError:
167 + pass

I think this could be simplified to:

if not load_rules.get(f, True):
 trace.mutter(...)
 continue

Also, I think this:
22 + def get_plugin_load_rules(self):
23 + """Return the plugins section."""
24 + if 'PLUGINS' in self._get_parser():
25 + values = self._get_parser()['PLUGINS']
26 + return dict([(k, not(v in ['off', 'disable', 'false']))
27 + for k, v in values.items()])
28 + else:
29 + return {}
30 +

1) Should probably use 'v.lower()'
2) Maybe should use 'ui.bool_from_string()'.

I don't really like allowing garbage settings to have 'default' results, rather than failing and telling the user that there is a problem with their configuration.

review: Needs Fixing

« Back to merge proposal