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

[Jon Schutz]

On Mon, 2008-04-21 at 19:16 +0100, Matt S Trout wrote:
> 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.

Indeed it is a compromise.  PBP says don't use AUTOLOAD, but for all the
reasons it gives for not using it, it could probably have a footnote
saying something like "If you're just putting in AUTOLOAD to support a
deprecated interface that's not going to be supported in the next major
revision, and the lifetime is pretty short, and nobody you know of is
actually using that deprecated interface anyway, then it's probably OK -
at least as a compromise".  That's what it says in my copy of PBP, I
think.

> 
> 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 :)

No problems, if that's what the Catalyst standard says; I must have
missed it.  Where is it?  I'd like to consult it on a number of
matters... please post the link.

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

The fact that it's supposedly already in a stage of "completely broken"
kind of undermines that theory.

I'm quite aware that I've spent more time debating the point than it
would have taken just to do this nugatory work, but then we wouldn't be
having this interesting discussion.  Can we put a timescale on it?  What
is the plan for release of 5.7013 and/or 5.80?

-- 

Jon Schutz                        My tech notes http://notes.jschutz.net
Chief Technology Officer                        http://www.youramigo.com
YourAmigo