Code review comment for lp:~jelmer/bzr/plugin-refactor

On Sat, 2009-07-18 at 21:15 +0000, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/plugin-refactor into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> The attached patch refactors the plugin loading code a bit so we
> remember the suffix tuple when finding plugins so we can use it for
> loading them later. This saves a few stats, and will make it possible
> to load individual plugins in the future (this is what I hope to work
> on next).

I think this may be buggy/introduce bugs. You change the load logic
from:
calculate the plugin path
calculate names to load
import them all

to
calculate the plugin path
calculate names-in-directories-to-load
import-those-names

You also catch an ImportError and squelch it somewhere where it looks
like we'd want to know that the error occurred.

The patch is also going away from using 'import' to using
'imp.find_module' + 'imp.load_module' - we had bugs in the past
*because* we used those rather than using the main python import code
path.

This doesn't seem like a good conceptual idea to me. I don't want to
stop you achieving what you need to, but - why can't you just put the
plugin you want on the plugins path.

 review -1