Re: P::C or PPI bug?

2006-11-08 Thread Christopher H. Laco
Chris Dolan wrote:
 On Nov 7, 2006, at 6:15 PM, Christopher H. Laco wrote:
 
 I just wanted to get some thoughts on this before I filed a bug report
 with either PPI or Perl::Critic:

 I'm going through and testing all of my *.t files for RequireTestLabels.
 I was humming right along when I ran into an oddity. The newest
 Perl::Critic 0.21 + PPI 1.118 complains that the tests don't have
 labels, yet they clearly do.

 After some tinkering, I stumbled across the culprits:

 This causes a RequireTestLabel failure:

 is_deeply([$storage-primary_columns], [qw/id otherpk/], 'return DBIC
 primary keys from schema');

 other is_deeply tests pass, like the one before it:

 is($storage-_primary_columns, undef, 'no primary columns defined');

 I looked through RT, and I don't see anything that really points to
 anything on this one in either dist.

 Ideas?
 
 This is fixed in the latest PPI developer release v1.199_xx.  The array
 constructor stuff should be perfect.  The hash constructor parsing still
 needs some work to further disambiguate it from anonymous blocks. (like
 map, grep, eval, sort, etc)
 
 It never hit RT because I just fixed it myself instead of reporting.
 
 Chris

I gave the latest PPI + Perl::Critic SVN a shot late last night. That
problem is indeed fixed for me, but another on cropped up with
UseStrict, even through I am, and it only happens in 2 of 30 files.

As soon as I get ouf of my meetings this morning, I'll update on the
local box and have another go and post the actual error.

-=TheOtherChris



signature.asc
Description: OpenPGP digital signature


Re: P::C or PPI bug?

2006-11-08 Thread Chris Dolan

On Nov 7, 2006, at 11:37 PM, Adam Kennedy wrote:

I'm not sure you can reliably test that though, not to 100% anyway,  
given the problem of implicit params.


is( foo $bar, $baz, $expected );

which means

is( foo($bar, $baz), $expected );

Yes, his specific case is ok, but I think you need to be a bit  
cunning about how you check...


Well, the former is just bad coding style.  So if Perl::Critic or PPI  
misinterprets it that's the coder's fault, not the tool's fault!  :-)


One of the best lessons in Perl Best Practices: if your code is  
ambiguous without looking at the core Perl docs (or source code!),  
then it should be rewritten to be less ambiguous.


Chris

--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Clotho Advanced Media, Inc. - Creators of MediaLandscape Software  
(http://www.media-landscape.com/) and partners in the revolutionary  
Croquet project (http://www.opencroquet.org/)





P::C or PPI bug?

2006-11-07 Thread Christopher H. Laco
I just wanted to get some thoughts on this before I filed a bug report
with either PPI or Perl::Critic:

I'm going through and testing all of my *.t files for RequireTestLabels.
I was humming right along when I ran into an oddity. The newest
Perl::Critic 0.21 + PPI 1.118 complains that the tests don't have
labels, yet they clearly do.

After some tinkering, I stumbled across the culprits:

This causes a RequireTestLabel failure:

 is_deeply([$storage-primary_columns], [qw/id otherpk/], 'return DBIC primary 
 keys from schema');

other is_deeply tests pass, like the one before it:

 is($storage-_primary_columns, undef, 'no primary columns defined');

I looked through RT, and I don't see anything that really points to
anything on this one in either dist.

Ideas?

-=Chris



signature.asc
Description: OpenPGP digital signature


Re: P::C or PPI bug?

2006-11-07 Thread Chris Dolan

On Nov 7, 2006, at 6:15 PM, Christopher H. Laco wrote:


I just wanted to get some thoughts on this before I filed a bug report
with either PPI or Perl::Critic:

I'm going through and testing all of my *.t files for  
RequireTestLabels.

I was humming right along when I ran into an oddity. The newest
Perl::Critic 0.21 + PPI 1.118 complains that the tests don't have
labels, yet they clearly do.

After some tinkering, I stumbled across the culprits:

This causes a RequireTestLabel failure:

is_deeply([$storage-primary_columns], [qw/id otherpk/], 'return  
DBIC primary keys from schema');


other is_deeply tests pass, like the one before it:


is($storage-_primary_columns, undef, 'no primary columns defined');


I looked through RT, and I don't see anything that really points to
anything on this one in either dist.

Ideas?


This is fixed in the latest PPI developer release v1.199_xx.  The  
array constructor stuff should be perfect.  The hash constructor  
parsing still needs some work to further disambiguate it from  
anonymous blocks. (like map, grep, eval, sort, etc)


It never hit RT because I just fixed it myself instead of reporting.

Chris

--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Clotho Advanced Media, Inc. - Creators of MediaLandscape Software  
(http://www.media-landscape.com/) and partners in the revolutionary  
Croquet project (http://www.opencroquet.org/)





Re: P::C or PPI bug?

2006-11-07 Thread Adam Kennedy
I'm not sure you can reliably test that though, not to 100% anyway, 
given the problem of implicit params.


is( foo $bar, $baz, $expected );

which means

is( foo($bar, $baz), $expected );

Yes, his specific case is ok, but I think you need to be a bit cunning 
about how you check...


Adam K

Chris Dolan wrote:

On Nov 7, 2006, at 6:15 PM, Christopher H. Laco wrote:


I just wanted to get some thoughts on this before I filed a bug report
with either PPI or Perl::Critic:

I'm going through and testing all of my *.t files for RequireTestLabels.
I was humming right along when I ran into an oddity. The newest
Perl::Critic 0.21 + PPI 1.118 complains that the tests don't have
labels, yet they clearly do.

After some tinkering, I stumbled across the culprits:

This causes a RequireTestLabel failure:

is_deeply([$storage-primary_columns], [qw/id otherpk/], 'return DBIC 
primary keys from schema');


other is_deeply tests pass, like the one before it:


is($storage-_primary_columns, undef, 'no primary columns defined');


I looked through RT, and I don't see anything that really points to
anything on this one in either dist.

Ideas?


This is fixed in the latest PPI developer release v1.199_xx.  The array 
constructor stuff should be perfect.  The hash constructor parsing still 
needs some work to further disambiguate it from anonymous blocks. (like 
map, grep, eval, sort, etc)


It never hit RT because I just fixed it myself instead of reporting.

Chris

--Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Clotho Advanced Media, Inc. - Creators of MediaLandscape Software 
(http://www.media-landscape.com/) and partners in the revolutionary 
Croquet project (http://www.opencroquet.org/)