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

Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

> * We don't want to do all the work to load config each time this function is
> called

This proposal doesn't materially change the work that this function was already doing. If it's desired to optimize the performance, that could be a separate change.

> * We still need a generic don't multiply warn for various things

Yes, that need will still exist after this change.

> 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.

This attribute is only checked inside this function and shouldn't affect any other warning.

> Is there any reason to refresh the config dir mid process execution?

I have no idea, but this function certainly gets called many times during the execution of a single command such as "brz status", at least in my testing. In fact, this function gets called at plugin import time, before logging is even configured.

« Back to merge proposal