Re: perlcritic

On 08/31/2015 11:57 PM, Peter Eisentraut wrote:
> We now have 80+ Perl files in our tree, and it's growing.  Some of those
> files were originally written for Perl 4, and the coding styles and
> quality are quite, uh, divergent.  So I figured it's time to clean up
> that code a bit.  I ran perlcritic over the tree and cleaned up all the
> warnings at level 5 (the default, least severe).


I don't object to this. Forcing strict mode is good, and I think I 
stopped using bareword file handles around 17 years ago. OTOH, I don't 
care that much about the two argument form of open(), and I doubt we 
gain a heck of a lot by changing it to the three argument form. It seems 
to me more a matter of stylistic preference than any significant 
technical improvement. In some cases it arguably leads to less clarity, 
e.g. this doesn't seem to add clarity:
   -    open $fh, "./$tmp |" or die;   +    open $fh, '<', "./$tmp |" or die;


Note also that in some cases all that's happened is that it's added 
comments so that future percritic runs will ignore what it's complaining 
about. We should look at those cases and annotate them (either we're 
happy the way it is or we should fix them)

In pgindent, there are a couple of uses of eval that are mine 
originally. At least one should be able to be replaced, thus:
    require LWP::Simple;    LWP::Simple->import('getstore');


I'd have to look at the other one more closely.

cheers

andrew