[Catalyst] Why does $c->stats require -Debug flag?

[Matt S Trout]

On Mon, Apr 21, 2008 at 11:49:56AM +0930, Jon Schutz wrote:
> On Sun, 2008-04-20 at 15:15 +0100, Matt S Trout wrote:
> 
> > So far as I can see, all we really need to do is supply a proxy of the
> > common Tree::Simple method from the C::Stats object through to $self->{tree}
> > and we're done. That'll provide compatibility with obvious usages without
> > adding any significant compatibility overhead.
> > 
> > I was hoping Jon would do this because he was the original author of C::Stats
> > and could see any subtleties that needed paying attention to that I've
> > missed.
> > 
> 
> I would have to review the pre-5.7012 code but from memory there were
> some differences in when internal fields in the tree were set, so
> returning $self->{tree} will stop crashes but there may be some side
> effects, such as inaccuracies in the resulting reports.

Well, if there is you can make the warning mention that.
 
> Trouble with explicitly proxying the methods is that Tree::Simple has
> many methods and who knows how many people have used what out there (I
> suspect, very few and very little, but who knows?).

So? That's just a for() loop setting up *{$name} = sub { ... } entries.
 
> You've heard my objections on principle and resistance due to minimal
> residual impact in practice, but if one were to fix it, I suppose a
> simple compromise would be something like this (untested):

That's not a compromise, that's an AUTOLOAD, which is poor coding practice
when you know what methods the object on the other side exists.

I'm aware you object on principle; however I've stated very clearly why I
believe your objections are incorrect and since you're contributing to
Catalyst I'd ask that you follow the current Catalyst standards for
backwards compatibility even if you disagree, just the same as you'd do
for coding style and other matters of opinion. If I ever contribute to one
of your projects I'll happily return the favour :)

Please can you do a specific setup, with tests; I'd suggest using
Class::Inspector to pull the list of methods and to proxy all those that
don't currently exist in your class.

Then we can have a warning included and happily throw these methods out in
5.80; the point is that people's code shouldn't go from "fully working" to
"completely broken" without a stage of "still works but warns them they're
doing it wrong" first (and note that if we'd called the method $c->_stats
I'd agree with you that it was private and we can deprecate it at will. but
we didn't. such is life)

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director                    http://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/            http://www.shadowcat.co.uk/servers/