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

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Vincent Ladeuil wrote:

> That's not in particular, windows is the only platform where we
> have this problem.

The issues I'm looking to solve impact all platforms IMO. Even on Linux,
a user may wish to disable a plugin that's installed system-wide and
they may not have permissions to touch the system paths where that
plugin is installed.

> PATH is as magic as BZR_PLUGIN_PATH, I don't think it's relevant
> here.

I think you and I are trying to solve very different Use Cases. My
drivers are:

1. More batteries should be included out-of-the-box in installers. Some
users will want or need to disable some of those batteries and they'll
most likely want to do it "forever". In some cases, plugins will be
broken and they'll want to turn them off. Or they'll hit and bug and
want to see whether having certain plugins installed fix the problem or
otherwise. Others may simply prefer a smaller footprint, e.g. less
entries in the list of commands when running qrun, less parts that might
break or introduce errors, etc.

2. There should be a feature in qplugins for enabling/disabling plugins.
That data ought to be permanently stored somewhere sensible like
bazaar.conf. An environment variable just doesn't cut it.

My focus is end-users, e.g. someone who clicks on a plugin in qplugins
and is given the option to disable it or to select a particular instance
of it found on the plugins path. Your focus seems to be test runners and
plugin developers where the needs are far more short term.

> You haven't even reply to the other conses....

Fair enough:

* "adds complexity" is a motherhood statement. Of course it does.
  So do 99% of other new features.

* "Developers can't give an explicit path". Agreed. I could
  fix that. OTOH, current mechanisms (like symlinks) works fine
  right now so it didn't feel worth it.

* "Restriction to a single run". I like your idea of environment
  variables for that case. You could just as nicely solve it by
  using an environment variable pointing to a custom bazaar.conf, yes?
  In that case, you could more easily test any custom settings in
  bazaar.conf, not just custom plugin paths.

> And you're already bringing back a feature which should clearly
> be addressed by using BZR_PLUGIN_PATH.

I'm sorry but that statement is simply not true. For your Use Cases,
environment variables sound fine. For mine, they are the wrong choice.

Having multiple mechanisms may indeed be the right answer here, given
the different problems we're trying to address. Regardless, having zero
mechanisms isn't the right number. :-)

Ian C.

« Back to merge proposal