From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: perlcritic |
Date: | 2015-09-01 13:58:17 |
Message-ID: | 55E5AEF9.2050504@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2015-09-01 14:05:44 | Re: WIP: About CMake v2 |
Previous Message | Robert Haas | 2015-09-01 13:53:34 | Re: icc vs. gcc-style asm blocks ... maybe the twain can meet? |