Re: [mp1 Patch] Looking for magic in Apache->request

> > I'm not sure why that code was there. So I'm not so confident on removing it. 
> > mp1's test suite is too poor for regression testing.
>
>
> Yeah, you can say that again. I dunno if it would be eventually worth it
> to port mp1's tests to Apache::Test and try and achieve better
> covereage. But once again, how long are we planning mp1 will stick
> around ? Would it be worth it ? (not for 1.28 for sure)

mp1 is here to stay for a while. However if you don't change its guts you are 
safe from breaking things. Millions of installations are a good test suite ;)

It's definitely not worth spending time to port the test suite, since we have 
too few resources to spend on bringing mp2 to a production level. Once that's 
completed we can think of rewriting mp1's test suite.

> > > Index: lib/Apache/Registry.pm
> > > ===================================================================
> > > RCS file: /home/cvs/modperl/lib/Apache/Registry.pm,v
> > > retrieving revision 1.34
> > > diff -u -I$Id -r1.34 Registry.pm
> > > --- lib/Apache/Registry.pm      23 May 2002 04:21:07 -0000      1.34
> > > +++ lib/Apache/Registry.pm      28 May 2003 12:53:54 -0000
> > > @@ -33,13 +33,6 @@
> > >
> > > sub handler {
> > >     my $r = shift;
> > > -    if(ref $r) {
> > > -       $r->request($r);
> > > -    }
> > > -    else {
> > > -       #warn "Registry args are: ($r, @_)\n";
> > > -       $r = Apache->request;
> > > -    }
> >
> > This has to do with subclasses, I suppose. Which seem to allow calling 
> > handler() with no real $r object. I'd keep it in.
>
>
> Hrm, not super sure about it, but I just didn't like the fact that with
> my patch applied, the modperl_request_rec_sv would be set for every
> single Apache::Registry requests, which is quite a waste, since it does
> not set it to a subclass of 'Apache'. Just would prefer avoiding
> creation of useless SV unless it's really neded. How about:

but since everybody seems to agree that your patch shoudln't be applied now, 
we don't have to worry about it, no?

> <untested patch>
>  -    if(ref $r) {
>  +    if(ref($r) ne 'Apache') {
> </untested patch>

> > What's Apache::CGI? If you remove it, will it break that module?
>
>
> For the love of me, I couldn't find it anywhere, search.cpan.org or
> google, so ...

It must be autovivified on a certain moon phase ;)

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@xxxxxxxxxx http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com