> I'm all in favor. > > I'm doing more work on plugins right now and have a bunch more changes in the > pipes. Most of the changes revolve around several practices which I think all > plugins should adhere to: > > One thing that would help is if you could split your changes into multiple > pull requests -- if we're going to try using github as our mechanism, every > pull request should be for one "feature". There's a lot of your changes > pending right now which are no-brainers. Typo fixes or other cleanups -- but > to apply those without everything else in your tree requires a cherry-pick, > which IIRC will wreak havoc with your tree when you try to rebase. (Or maybe > not, and I should just start picking and getting the easy ones out of the way > so we can focus on the others.)
I don't know the repercussions of cherry picking. Perhaps this is as good a time as any to give it a try and find out? > store operational results in notes > > > Yes. > > use a 'reject' option: instead of rejecting messages that fail a test, offer > an option for what to do. > (ex: require_resolvable_fromhost becomes resolvable_fromhost with a > reject option > this allows information to be collected from a plugin when it would > otherwise be unusable > > > I'm 50/50 on this one. On one hand, it provides more configuration options > and flexibility. On the other hand, it requires people to set more > configuration options and increases the complexity of the configuration and > may make the simple cases harder. Right now, having no options means many plugins disabled is a reasonable default. Being "all or nothing" makes many of the plugins of less value than they could be. As it stands, there's often no way to answer questions like, 'how much of my ham will fail this test?' or 'will I lose valid mail using plugin X?' It's completely nebulous. OTOH, if the plugin is enabled by default, with a default reject=no policy (same as the current "disabled by default" implicit policy), then the plugin is at least logging data that will allow questions like that to be answered. > I also think logging anything more severe than a WARN event should have the > option to automatically email a sysadmin. > > I'm not a fan of tying in auto-emailing of severe notifications. (I've got a > great story about how I took down a corporate mail system 7 years ago that > way...) That's why it should be an option. :-) Disabled can be the default, to make you happy, and I can enable it. To prevent inadvertent pager/alert floods (ie, the classic Big Brother alerting problem, from Way Back In The Day), a 'warn once per X minutes' error alerting policy should be the minimum bar to clear for implementation. > I'm working on a "Qpsmtpd Big Picture" document that contains a flowchart of > a sample connection, a catalog of all the plugins, what each plugin > does/sets/changes, and what information is made available to subsequent > plugins via notes. It took me quite a while to wrap my head around this, and > that learning curve presents a significant barrier to entry for potential > qpsmtpd users. > > Speaking of opening things up to major changes and cleanups, I have to say > I'm not terribly fond of the check_ prefix on so many of the plugin names. I > think I get the idea behind it, but many of the plugins do more than check. > Many also reject messages that fail their checks, by default. Why not just > drop the check_ prefix entirely, and have the plugin name be shorter? > > Same goes for plugins like require_resolvable_fromhost. I think its better > to make the plugin resolvable_fromhost, and then offer config options that > govern whether the plugin merely checks, or rejects based on the outcome of > the test. > > This goes with your configuration options direction above. I can definitely > get into a naming bikeshed color discussion with both feet -- but will > restrain. The bigger issue for me is backwards compatibility. If we change > the name of any core plugins, that's going to break users when they try and > upgrade. It's pretty easy to leave placeholder plugin wrappers in place, but > users will still break if the functionality changes. If you're interested in > driving all these changes, maybe we should consider a new target: qpsmtpd2 or > something. (I'm not trying to discourage you -- I'm very happy someone is > interested in this -- I just don't want to break the current users.) Which is more important, keeping existing users, or reaching new ones? To reach new users, qpsmtpd needs to lower the barriers to entry. It needs to become better. Better documentation, standardized behavior of plugins, a 'big picture' diagram of how mail flows through the system, documentation techniques to diagnose basic problems, how twiddling certain knobs will affect mail flow, which phase of the SMTP connection the plugins fire in, etc. These and other changes can make qpsmtpd more approachable to those who aren't perl hackers. A potential user of qpsmtpd needs to feel like they understand it before they'll trust or use it. Right now, understanding it is a large barrier to entry. I laud the desire to not impact upgrading users, but why might people want to upgrade in the first place? Because the new version is better, right? So why isn't better the standard for whether or not changes are committed? If the new version is better, users will upgrade, even if there's effort involved. I'm not saying we should ignore the legacy issues. Just that there are ways to handle legacy changes. Is removing the check_ plugin prefix better or not? Especially when the plugin is rejecting mail if enabled? If renaming the plugin is better, then finding a way to lessen the impact on existing users is merely a programming exercise. I think leaving placeholder plugin wrappers is a bad idea because it makes it harder on humans to make sense of the contents of the plugins directory. A simple and onerous way to handle upgrades is an UPGRADE. Another way is to add a Plugin/Legacy.pm module that maps legacy to new plugin names. If a plugin name is encountered in a config file and it doesn't exist in the plugins directory, then consult Legacy.pm for a mapping that calls the new plugin with the legacy behavior. For good measure, log an error alerting the admin to update their config file. Matt
