On Sat, 2004-02-28 at 14:14, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> > Finally got around implementing Apache::PerlSections->(dump|store);
> >
> > Comments ?
> >
> > Index: lib/Apache/PerlSections.pm
> > ===================================================================
> > RCS file: /home/cvs/modperl-2.0/lib/Apache/PerlSections.pm,v
> > retrieving revision 1.2
> > diff -u -I$Id -r1.2 PerlSections.pm
> > --- lib/Apache/PerlSections.pm 19 Dec 2003 01:17:31 -0000 1.2
> > +++ lib/Apache/PerlSections.pm 28 Feb 2004 02:38:56 -0000
> > @@ -24,6 +24,10 @@
> > sub directives { return shift->{'directives'} ||= [] }
> > sub package { return shift->{'args'}->{'package'} }
> >
> > +our @saved;
>
> why do you need a global here? won't my() work?
Yes, you are correct, it's a artefact leftover from a previous
implementation.
>
> > Index: lib/Apache/PerlSections/Dump.pm
> > ===================================================================
> > RCS file: lib/Apache/PerlSections/Dump.pm
> > diff -N lib/Apache/PerlSections/Dump.pm
> > --- /dev/null 1 Jan 1970 00:00:00 -0000
> > +++ lib/Apache/PerlSections/Dump.pm 28 Feb 2004 02:38:56 -0000
> > @@ -0,0 +1,79 @@
> > +package Apache::PerlSections::Dump;
> > +
> > +use strict;
> > +use warnings FATAL => 'all';
> > +
> > +our $VERSION = '0.01';
> > +
> > +use Apache::PerlSections;
> > +our @ISA = qw(Apache::PerlSections);
>
> i think 'use base' is pretty safe to use with 5.6.1+
>
> > +use Data::Dumper;
> > +
> > +sub package { return shift->saved }
> > +sub save { return }
> > +sub post_config { return }
>
> What's the point of 'return'? Don't you want to have something like:
>
> sub save {
> warn "this (save) is a virtual method: you need to implement it";
> return
> }
Actually, no, it's quite the opposite. Those empty functions are there
to disable these actions in the parent class. A::P::Dump isa
A::PerlSections and really just overrides a few things, like
dump_item(), but in the parent class, for instance, post_config() is the
sub that ends up calling Apache->server->add_config() and in the case of
the dumping subclass, we want to make that a NOP
I guess a comment to explain that would help a bit.
I've also finished documenting store/dump, so I'll re-send a patch
including that shortly.
> or similar? may be using caller to get the class name... same for the other
> virtual method... write a wrapper?
>
> the rest looks good, Philippe!
Allrighty!
> __________________________________________________________________
> Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
> http://stason.org/ mod_perl Guide ---> http://perl.apache.org
> mailto:[EMAIL PROTECTED] 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
