Torsten Foertsch wrote: > Hi Geoff, > > On Tuesday 09 October 2007 19:19, Geoffrey Young wrote: >> if you're really confused, see the commits when I added Apache::MPM >> >> http://marc.info/?l=apache-modperl-cvs&m=106978727408877&w=2 >> http://marc.info/?l=apache-modperl-cvs&m=106978912311523&w=2 >> >> the bulk of the main commit consists of the tests, but the interesting >> changes are at the bottom. if you just follow that pattern you should >> be able to add the module of your choice. > > I think I did it. Please have a look at the attached patch to see if it is > all > that is needed to be done.
It looks pretty good, awesome! > The patch is a bit edited to suppress other changes in xs/. So I don't think > it would apply to trunk without warnings. It did apply clean for me. The only question I have is wherever it's necessary to also expose the following 2 items: modperl_interp_pool_t * | IV PerlInterpreter * | IV Especially in the current patch, what you get out will pretty much be a useless blessed scalar you can't do anything with. Apart that, looks great. At this point, I'd be happy to see it go in, without documentation even, but at least with a minimal set of tests, something like: my $int = ModPerl::Interpreter->current; ok($int); is($int, 'ModPerl::Intereter'); ok($int->mip); ok($int->refcnt); [...] To at least cover normal usage cases. Good work ! ------------------------------------------------------------------------ Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5 http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
signature.asc
Description: OpenPGP digital signature
