Thread: perlcritic script
Here's a small patch to add a script to call perlcritic, in the same way that we have a script to call perltidy. Is also includes a perlcriticrc file containing a policy to allow octal constants with leading zeros. That's the only core severity 5 policy we are currently no in compliance with. We should probably look at being rather more aggressive with perlcritic. I've made the buildfarm client code compliant with some exceptions down to severity level 3. Here are the profile exceptions: [-Variables::RequireLocalizedPunctuationVars] [TestingAndDebugging::ProhibitNoWarnings] allow = once [-InputOutput::RequireBriefOpen] [-Subroutines::RequireArgUnpacking] [-RegularExpressions::RequireExtendedFormatting] [-Variables::ProhibitPackageVars] [-ErrorHandling::RequireCarping] [-ValuesAndExpressions::ProhibitComplexVersion] [InputOutput::ProhibitBacktickOperators] only_in_void_context = 1 [-Modules::ProhibitExcessMainComplexity] [-Subroutines::ProhibitExcessComplexity] [-ValuesAndExpressions::ProhibitImplicitNewlines] [-ControlStructures::ProhibitCascadingIfElse] [-ControlStructures::ProhibitNegativeExpressionsInUnlessAndUntilConditions] [-ErrorHandling::RequireCheckingReturnValueOfEval ] [-BuiltinFunctions::ProhibitComplexMappings] There are also 21 places in the code with "no critic" markings at severity 3 and above. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 5/8/18 13:57, Andrew Dunstan wrote: > + # take executable files that file(1) thinks are perl files > + find . -type f -perm -100 -exec file {} \; -print | > + egrep -i ':.*perl[0-9]*\>' | How portable is that? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: > On 5/8/18 13:57, Andrew Dunstan wrote: >> + # take executable files that file(1) thinks are perl files >> + find . -type f -perm -100 -exec file {} \; -print | >> + egrep -i ':.*perl[0-9]*\>' | > How portable is that? Well, it's the same code that's in pgperltidy ... but I agree that it's making a lot of assumptions about the behavior of file(1). regards, tom lane
On 5/8/18 16:51, Tom Lane wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> On 5/8/18 13:57, Andrew Dunstan wrote: >>> + # take executable files that file(1) thinks are perl files >>> + find . -type f -perm -100 -exec file {} \; -print | >>> + egrep -i ':.*perl[0-9]*\>' | > >> How portable is that? > > Well, it's the same code that's in pgperltidy ... but I agree that > it's making a lot of assumptions about the behavior of file(1). OK, but then it's not a problem for this thread. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Moin, On Tue, May 8, 2018 5:03 pm, Peter Eisentraut wrote: > On 5/8/18 16:51, Tom Lane wrote: >> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >>> On 5/8/18 13:57, Andrew Dunstan wrote: >>>> + # take executable files that file(1) thinks are perl files >>>> + find . -type f -perm -100 -exec file {} \; -print | >>>> + egrep -i ':.*perl[0-9]*\>' | >> >>> How portable is that? >> >> Well, it's the same code that's in pgperltidy ... but I agree that >> it's making a lot of assumptions about the behavior of file(1). > > OK, but then it's not a problem for this thread. If I'm not mistaken, the first line in the "find" code could be more compact like so: find . -type f -iname '*.p[lm]' (-print is default, and the -name argument is a regexp, anyway. And IMHO it could be "-iname" so we catch "test.PM", too?). Also, "-print" does not handle filenames with newlines well, so "-print0" should be used, however, this can be tricky when the next step isn't xarg, but sort. Looking at the man page, on my system this would be: find . -type f -name '*.p[lm]' -print0 | sort -u -z | xargs -0 ... Not sure if that is more, or less, portable then the original -print variant, tho. Best regards, Tels