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

Revision history for this message
Robert Collins (lifeless) wrote :

On Tue, 2010-01-05 at 01:06 +0000, Martin Pool wrote:

> >> I'd like to not add ad-hoc environment variables for things that could
> >> reasonably be configured elsewhere. Let's put it into the config
> >> system and then have a way to take config from the environment.
> >
> > This isn't an adhoc environment variable - it already exists.
>
> s/add variables/add features controlled by environment variables

I think this is an intereseting goal, but setting config options from
the environment could be pretty complex (in terms of UI); we already
support controlling plugin loading from the environment, so I think its
entirely appropriate *in this case* to control it from the existing
variable - that or we should be aiming to deprecate this (and other)
variables.

Put another way: saying that we shouldn't do this from the existing
variable makes making this feature usable from an environment variable
hostage to an unwritten, unspecified (and AFAIK) undiscussed feature.

All the uses I've heard of up until this bug for precise controlling of
plugin paths have been for testing in precommit hooks, an environment
where using the configuration to control it will be awkward at best
(cannot use the branch config, shouldn't be making persistent changes to
the global config)

-Rob

« Back to merge proposal