On May 8, 2012, at 9:39 AM, Steve Kemp wrote:

> On Tue May 08, 2012 at 09:29:57 -0700, Robert Spier wrote:
> 
>>> +use strict;
>>> +use warnings;
>>> +
>>> +use base 'Qpsmtpd::Plugin';
>>> +use Qpsmtpd::Constants;
>> 
>> All of these except for use warnings come for free with any plugin.
>> They're not required here.
> 
> I have to say that if we're going to do a mass-shuffle in the near
> future to cleanup whitespace/POD/etc adding these lines in globally
> would be wonderful. 

Aye, my thoughts exactly.  In the core code, I think its less of an issue. In 
the plugins, where authors have widely varying perl skills, I believe it's far 
better to make those declarations explicit.

> Having all implicit modules included allows you to run plugin files
> through "perl -c" without false warnings. 

Which is precisely why I've been adding them. We are moving towards a 
comprehensive and robust test suite, and this is one of the baby steps that 
enables it. 

Coding in paragraph sized subs that can be easily tested is another that I 
think we should require for any future code additions. Not having to rewrite 
modules and plugins to add basic tests would make writing the tests easy enough 
that more people might actually do it. 

> [FWIW I'd agree moving the code to a central location is good, moving it into 
> ::Plugin is a bad plan.]


I didn't like putting in Qpsmtpd::Plugin either. Especially not since 
Qpsmtpd::Plugin it declared and behaves differently in the test suites, which 
necessitated putting the code in there too. 

How about Qpsmtpd::Plugin::auth?

That namespace is already the parent of the auth plugins. 

Matt

Reply via email to