httpd-test-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Geoffrey Young <ge...@modperlcookbook.org>
Subject Re: cvs commit: httpd-test/perl-framework/Apache-Test/lib/Apache TestConfig.pm
Date Thu, 23 Oct 2003 18:57:50 GMT


Stas Bekman wrote:
> Geoffrey Young wrote:
> 
>>
>>>> I should have noticed this before, but all the mod_perl foo in 
>>>> TestConfig really belongs in TestConfigPerl (including the stuff 
>>>> that was there before this flurry of commits :)
>>>>
>>>> it's too late for 1.05 at this point (which I hope to release on 
>>>> monday or tuesday) but right afterward I'm going to clean up a bit 
>>>> and remove all traces of mod_perl from TestConfig.
>>>
>>>
>>>
>>>
>>> Why? This code was there all the time and it has nothing to do with 
>>> mod_perl and needed for any other perl project. Your quite shows like 
>>> it wasn't added, but it wasn't, you have removed the - lines, which 
>>> have shown where it existed before.
>>
>>
>>
>> I'm not arguing about whether it was there before and you were just 
>> following suit.  this isn't necessarily about the changes you just made.
> 
> 
> ok

sorry you got the wrong impression :)

> 
>> what I'm saying is that we have the TestConfigPerl class for 
>> configuring mod_perl specific widgets and TestConfig should probably 
>> be as non-httpd-core-module neutral as we can make it.
> 
>  >
> 
>> so, stuff like IS_MOD_PERL_2, IS_MOD_PERL_2_BUILD, and PerlRequire 
>> shouldn't be there conceptually.  it just makes it harder to extend 
>> and maintain when there isn't a nice separation :)
> 
> 
> Sure, so instead of:
> 
>   if (IS_MOD_PERL_2_BUILD || $ENV{APACHE_TEST_LIVE_DEV}) {
> 
> it should then say:
> 
>   if ($self->should_include_live_dev) {
> 
> which can be subclassed in TestConfigPerl with IS_MOD_PERL_2_BUILD 
> moving there. so the superclass (TestConfig) goes:
> 
>   sub should_include_live_dev { return $ENV{APACHE_TEST_LIVE_DEV} ? 1 : 0 }
> 
> and TestConfigPerl:
> 
>    sub should_include_live_dev {
>       return IS_MOD_PERL_2_BUILD || shift->SUPER::should_include_live_dev;
>    }
> 
> does this sound reasonable?

yup, that's pretty much what I had in mind.  there are a few other places 
where the fix might not be that simple, but we've both got the same idea at 
least :)

that TestConfigPerl isn't a real subclass might pose some issues, but we 
might be able to fix that as well :)

anyway, this is all after the release - it doesn't make sense to change it 
now that you've gotten the lib thing working.  we'll sort it all out in a 
few days.

--Geoff


Mime
View raw message