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

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

> There are two complicating factors here that this change does address:
>
> * We don't want to do all the work to load config each time this function is
> called
> * We still need a generic don't multiply warn for various things
Such a system is tricky for this case though, since we'd probably use 'suppress_warnings=config_dir' or somesuch in the config to suppress it.

> As a concrete suggestion, I think a better choice is storing the config on
> global state rather than having a warning_printed attr, which will also happen
> to make any warning only appear once.
>
> Is there any reason to refresh the config dir mid process execution?
Environment variables can be changed, e.g. by tests. It doesn't seem like this MP is making the overhead of determining the config dir much worse though. The code path is also specific to the fallback case where ~/.config/breezy doesn't exist, but ~/.bazaar does.

« Back to merge proposal