Code review comment for lp:~thomir-deactivatedaccount/bzr/add-global-config

+class SystemGlobalStore(LockableIniFileStore):
+
+ def __init__(self, possible_transports=None):
+ t = transport.get_transport_from_path(system_config_dir(),
+ possible_transports=possible_transports)
+ super(SystemGlobalStore, self).__init__(t, 'bazaar.conf')

The point of passing possible_transports is basically so that we can reuse some connection objects, and that's mostly important for remote transports, which this won't be at present. I think the current approach of manually passing it down is not working very well and we should probably look at putting it into the library state instead.

[tweak] However, for this patch, I don't think it's either used or particularly useful (since making new local transports is cheap), so you could just remove the parameter.

[suggestion] I'm not a big fan of creating new classes that only have different initial state, not different behaviour, so I would probably just make this a factory method.

Aside from that, this looks great. It could be good to get a review from vila too.