Hanno,
May I suggest you send these large patches to the list before
committing them so we can do a group code and design review?
In general, I don't think we want this change, as it adds
complexity to the common core that we try and avoid. The skip
functionality can be implemented using the plugin wrapping
functionality.
Some of the "is_loaded" stuff may be commonly useful.
I'd rather see a generalization (make it hookable, or just easily
replacable) of the plugin dispatch loop instead of adding
condition blocks for every feature we need in there.
The advantage of a hooked plugin-dispatch over the wrapped
(inherited) plugins is that you don't need to modify
config/plugins as much.
I'm borderline on saying: "revert this and lets discuss it and
decide if we want it".
In general, any change that adds specific functionality to the
core that could otherwise be implemented as a plugin -- especially
if most users aren't ever going to use it -- shouldn't go in the
core.
Some random code comments below:
> --- branches/0.3x/config.sample/plugins (original)
> +++ branches/0.3x/config.sample/plugins Sun Dec 31 03:07:32 2006
> @@ -12,6 +12,9 @@
> # from one IP!
> hosts_allow
>
> +# skip selected plugins for some hosts:
> +skip_plugins
This shouldn't be on by default in the basic simple configuration.
> --- branches/0.3x/lib/Qpsmtpd.pm (original)
> +++ branches/0.3x/lib/Qpsmtpd.pm Sun Dec 31 03:07:32 2006
> @@ -367,6 +367,11 @@
> $@ and warn("FATAL LOGGING PLUGIN ERROR: ", $@) and next;
> }
> else {
> + my $skip = $self->connection->notes('_skip_plugins');
Is this inside the loop intentionally?
> +sub escape_plugin {
> + my $self = shift;
> + my $plugin_name = shift;
> + # "stolen" from Qpsmtpd.pm
CODE DUPLICATION ALERT. This code now exists in two places, which
makes it likely that if you change one place you'll break another.
