Code review comment for lp://staging/~nmb/brz/warn-bazaar-directory

Revision history for this message
Martin Packman (gz) wrote :

Okay, looking over again there are two bits to address.

First is the warnings/stderr thing - which I think we're fine punting on to a later branch, so the output code is fine (but just use stderr directly, see below).

Second is we end up calling config.config_dir() and while it doesn't do *much* work, we do keep adding to it. So, we should dangle the result off the library state. We can come up with a nice idiom for that later, but for something like:

    existing_dir = getattr(breezy.global_state, 'config_dir', None)
    if existing_dir is not None:
        return existing_dir
    ...
    breezy.global_state = resolved_dir
    return resolved_dir

Note, this means you don't need/want the warn once logic.

review: Needs Fixing

« Back to merge proposal