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

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

Ok, after some wtf instants and some help from igc, I've fixed that and checked
the results in the sphinx generated html.

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

Added.

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

Well, I struggled a bit with _capture_deprecation_warnings (for symbol_versioning.warn) and callCatchWarnings (for warnings.showwarning) before realizing I wanted *trace.warning*.

So I agree we should provide an easier way :)
May be if all warnings, note, mutter was controlled by UIFactory and friends things will be clearer ?

Since that's the first time I know of that we need to capture trace.warning, I'd rather punt on this one.

I could make a further submission if you prefer, but *I*'d prefer some unification
of various warnings first (if only by providing more explicit names).