Code review comment for lp:~vila/bzr/411413-disable-plugin

23 +BZR_DISABLE_PLUGINS
24 +~~~~~~~~~~~~~~~~~~~
25 +
26 +Under special circumstances, it's better to disable a plugin (or
27 +several) rather than uninstalling them completely. Such plugins
28 +can be specified in the ``BZR_DISABLE_PLUGINS`` environment
29 +variable.
30 +
31 +In that case, ``bzr`` will stop loading the specified plugins and
32 +will raise an import error if you try to import them explicitly.
33 +
34 +Example:
35 +~~~~~~~~
36 +

The ReST syntax here looks strange, because "Example" will be another heading at the same level as BZR_DISABLE_PLUGINS. Just leave it out and put a :: after the word "explicitly".

Users may not understand what "if you try to import them explicitly" means - indeed it doesn't mean a lot at the external user level. So you could say they will error if for example another plugin depends upon them?

202 + self.warnings = []
203 + def captured_warning(*args, **kwargs):
204 + self.warnings.append((args, kwargs))
205 + self.overrideAttr(trace, 'warning', captured_warning)

It seems like there should already be a facility for this, but maybe not. Maybe you could lift it to a base class to save time?

Otherwise very good, thanks for fixing this!

bb:tweak