Code review comment for lp:~abentley/bzr/appenddir

Revision history for this message
Vincent Ladeuil (vila) wrote :

So, I've been working on this from a different angle:
- http://doc.bazaar.canonical.com/devnotes/configuration.html
- lp:~vila/bzr/new-config

This is a work in progress which still doesn't provide a working solution in core.

So basically I can't disapprove this proposal until I'm able to point to a working implementation but on the other hand, this proposal use a feature I'd like to get rid of (option and store policies) so approving it means I'll have to deprecate it in the future and provide a migration path.

On the third hand, I will need to provide a migration path for the existing policies which will require a yet *unknown* amount of work ;)

That being said:
- option expansion is (now) available and using {nickname} is very close to the 'appendir' policy,
- 'appenddir' fooled me (twice) as a policy name: it really add the *basename* of the branch/wt which indeed is a dir but I tend to think in terms of os.path API and read 'appenddir' as 'os.path.dirname'

Regarding policies, the plan is to define them once and only once for a given option, that means *outside* of the config files. This is my main issue with the current implementation.

review: Abstain

« Back to merge proposal