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 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. -- Bruce Momjian <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com When a patient asks the doctor, "Am I going to die?", he means "Am I going to die soon?"
# syntax https://github.com/schwern/Perl-Critic/blob/master/examples/perlcriticrc verbose = 7 severity = 1 # list from --profile-proto # --verbose 8 outputs critic labels # Write open $handle, $path instead of open($handle, $path). [-CodeLayout::ProhibitParensWithBuiltins] # Must run code through perltidy. [-CodeLayout::RequireTidyCode] # Write if($condition){ do_something() } instead of do_something() if $condition. [-ControlStructures::ProhibitPostfixControls] # Give every module a $VERSION number. [-Modules::RequireVersionVar] # Write @{ $array_ref } instead of @$array_ref. [-References::ProhibitDoubleSigils] # Always use the /s modifier with regular expressions. [-RegularExpressions::RequireDotMatchAnything] # Always use the /x modifier with regular expressions. [-RegularExpressions::RequireExtendedFormatting] # Always use the /m modifier with regular expressions. [-RegularExpressions::RequireLineBoundaryMatching] # Use character classes for literal meta-characters instead of escapes. [-RegularExpressions::ProhibitEscapedMetacharacters] # Don't write sub my_function (@@) {}. [-Subroutines::ProhibitSubroutinePrototypes] # Write q{} instead of ''. [-ValuesAndExpressions::ProhibitEmptyQuotes] # Don't use values that don't explain themselves. [-ValuesAndExpressions::ProhibitMagicNumbers]