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

On Thu, 2003-05-29 at 15:14, Stas Bekman wrote:
> >>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 ;)

Yeah, but this patch is a good example.

I want to stick with : "If it aint broken, don't fix it"

Only problem, it's broken. But it's hard to have any confidence in
patches like this one that touches 'guts' stuff.

I am just worried that these patches will stay out there for quite a
long time before they are eventually included.

As I am processing STATUS, I am hoping I'll be able to at least produce
a viable patch for most issues (or dismiss them). But on what basis
should we decide what does go in a 1.28-tobe and what stays a patch for
users to test?

> 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.

Good then, I won't even think about it for now

> >>>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
-- 
-- -----------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

signature.asc
Description: This is a digitally signed message part