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

> That also sounds sensible. Since the Stores are an internal API (we want people to be using the Stack classes, right?), I guess I should follow naming convention and call these "_get_global_store" etc?
>
> Also, I've renamed the GlobalStore to UserStore, and the "SystemGlobalStore" to just "GlobalStore", except that after the change above none of these will be visible, since they'll all come out of factory functions.
>
> BranchStore is a bit more difficult, as it uses the branch lock for locking. I could keep the BranchStore class as-is, or I could wrap that up inside a factory function as well, which would make everything look a lot neater, but I'm not sure what your coding standards are regarding nested classes....
>
> Finally, the win32utils unit tests fail, so I have some work to do there...

On naming:

We definitely shouldn't change the meaning of an existing name like
GlobalStore, even if it's not intended to be exposed. It's much
better for out-of-date code to get eg a NameError than to run the
wrong thing.

If the class really has actually different behaviour, like
BranchStore, it deserves to be a real different class. But if it's
basically the same class just with different constructor parameters, I
wonder if it's worthwhile. Perhaps for simplicity it is better just
to leave them all as classes. I may be getting a bit bikesheddy here
so don't block on it.

m