Re: perlcritic - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: perlcritic
Date
Msg-id 55E5AEF9.2050504@dunslane.net
Whole thread Raw
In response to perlcritic  (Peter Eisentraut <peter_e@gmx.net>)
Responses Re: perlcritic  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers

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




pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: icc vs. gcc-style asm blocks ... maybe the twain can meet?
Next
From: Tom Lane
Date:
Subject: Re: WIP: About CMake v2