On 2024-09-15 Su 9:28 PM, Bruce Momjian wrote:
On Sun, Sep 15, 2024 at 06:07:21PM -0400, Andrew Dunstan wrote:
On 2024-09-15 Su 4:36 PM, Bruce Momjian wrote:
On Sun, Sep 15, 2024 at 04:33:49PM -0400, Andrew Dunstan wrote:
I understand perfectly what the warning is about.
But the project's perlcritic policy is expressed at src/tools/perlcheck/
perlcriticrc. It's basically severity 5 plus some additions and one exception.
We shouldn't be imposing our personal perlcritic policies on the project.
The change isn't bad in itself, but there shouldn't be any expectation that we
will keep to this policy, as it's not required by project policy.
There is a huge mass of perlcritic issues in our perl code at severity 1 (over
13000 by my count). If we're going to clean them up (which I would oppose) we
should do it in a systematic way. It's hard to see why this one is special.
I guess it is special only because my policy is more strict so I wanted
my new code to match. Should I revert it?
Yes I think so.
Okay, done.
Is the very tiny improvement
not worth the churn in the code?
Yeah, I don't think it is.
FYI, the line that got me started was:
my ($tmpfilename) = $filename . ".tmp";
while I would write that with single quotes. Because mine uses single
quotes and version_stamp.pl uses double-quotes, I started to try to
unify the style.
I would normally write this as
my $tempfile = "$filename.tmp";
or possibly
my $tempfile = "${filename}.tmp";
There's a perl mantra that says There Is More Than One Way To Do It,
usually abbreviated to TIMTOWTDI, which perlcritic fights against to
some extent. I think the idea of unifying style beyond the official
project policy is an effort doomed to failure. Nobody is going to
maintain that consistency, and the project has rejected attempts to have
a stricter set of rules in the past.
I agree severity=1 generates many false warnings, and changing code
based on them can actually produce errors, but I do think there are some
safe ones like the single/double quote item we can improve.
Attached is my Perl critic file, where I do use severity=1, but turn off
various warnings.
Sure, or for something in between, there's the buildfarm policy at
<https://github.com/PGBuildFarm/client-code/blob/main/.perlcriticrc>
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
