It looks like you've addressed John's review nicely.
In doc/developers/xdg_config_spec.txt there's a typo:
> +%APPDATA%/Bazaar/2.0 and on Mac OS X, the directory is ~/.bazaaar. With the
You don't need quite so many “a”s in bazaar :)
> Note that the following descriptions do not apply
> +to Windows or Mac OS X which can and should use their own native configuration
> +locations.
True... although is ~/.bazaar the native location for Mac OS X? I'm not sure what
is expected on OS X, and improving the OS X experience is out of scope for this
patch, but this doc shouldn't be misleading about how good we are currently.
It would be nice to have tests for this, but I guess it's awkward to do. Hmm, I
suppose it's possible by overriding the environment variables for $HOME and/or
$XDG_CONFIG_HOME. TestConfigPath in bzrlib/tests/test_config.py is probably a
good starting point.
The only other comment I have is this is worth highlighting in What's New in 2.3
as well.
I think this should have some simple tests added before we merge it, so I'm voting
Needs Fixing, but basically this is looking pretty good.
It looks like you've addressed John's review nicely.
In doc/developers/ xdg_config_ spec.txt there's a typo:
> +%APPDATA% /Bazaar/ 2.0 and on Mac OS X, the directory is ~/.bazaaar. With the
You don't need quite so many “a”s in bazaar :)
> Note that the following descriptions do not apply
> +to Windows or Mac OS X which can and should use their own native configuration
> +locations.
True... although is ~/.bazaar the native location for Mac OS X? I'm not sure what
is expected on OS X, and improving the OS X experience is out of scope for this
patch, but this doc shouldn't be misleading about how good we are currently.
It would be nice to have tests for this, but I guess it's awkward to do. Hmm, I tests/test_ config. py is probably a
suppose it's possible by overriding the environment variables for $HOME and/or
$XDG_CONFIG_HOME. TestConfigPath in bzrlib/
good starting point.
The only other comment I have is this is worth highlighting in What's New in 2.3
as well.
I think this should have some simple tests added before we merge it, so I'm voting
Needs Fixing, but basically this is looking pretty good.