Re: cleaning perl code

On Thu, Apr 16, 2020 at 09:53:46AM -0400, Tom Lane wrote: > Andrew Dunstan writes: > > On 4/15/20 11:01 PM, Noah Misch wrote: > >> It would be an unpleasant surprise to cause a perlcritic buildfarm failure > >> by > >> moving a function, verbatim, from a non-strategic file to a strategic file. >

Re: cleaning perl code

> On Apr 16, 2020, at 2:07 PM, Andrew Dunstan > wrote: > > > On 4/16/20 11:12 AM, Alvaro Herrera wrote: >> On 2020-Apr-16, Hamlin, Garick L wrote: >> >>> With the old expression 'something;' would be stripped away. >>> Is that an issue where this this is used? Why are we parsing >>> thes

Re: cleaning perl code

On 4/16/20 11:12 AM, Alvaro Herrera wrote: > On 2020-Apr-16, Hamlin, Garick L wrote: > >> With the old expression 'something;' would be stripped away. >> Is that an issue where this this is used? Why are we parsing >> these headers? > These are files from which bootstrap catalog data is genera

Re: cleaning perl code

On 2020-Apr-16, Hamlin, Garick L wrote: > With the old expression 'something;' would be stripped away. > Is that an issue where this this is used? Why are we parsing > these headers? These are files from which bootstrap catalog data is generated, which is why we parse from Perl; but also where

Re: cleaning perl code

On 4/16/20 10:20 AM, Hamlin, Garick L wrote: > On Thu, Apr 16, 2020 at 08:50:35AM -0400, Andrew Dunstan wrote: >> It would also be more robust using non-greedy matching: > This seems more important. > I don't know how/where this is being used, but if it has input like: > > /* one */ > something;

Re: cleaning perl code

On Thu, Apr 16, 2020 at 08:50:35AM -0400, Andrew Dunstan wrote: > > It would also be more robust using non-greedy matching: This seems more important. I don't know how/where this is being used, but if it has input like: /* one */ something; /* two */ With the old expression 'something;' would

Re: cleaning perl code

Andrew Dunstan writes: > On 4/15/20 11:01 PM, Noah Misch wrote: >> It would be an unpleasant surprise to cause a perlcritic buildfarm failure by >> moving a function, verbatim, from a non-strategic file to a strategic file. >> Having two Perl style regimes in one tree is itself a liability. > Hon

Re: cleaning perl code

On 4/15/20 11:01 PM, Noah Misch wrote: > On Wed, Apr 15, 2020 at 03:43:36PM -0400, Andrew Dunstan wrote: >> On 4/14/20 4:44 PM, Alvaro Herrera wrote: >>> On 2020-Apr-14, Andrew Dunstan wrote: One of the things that's a bit sad is that perlcritic doesn't generally let you apply policies

Re: cleaning perl code

On Wed, Apr 15, 2020 at 03:43:36PM -0400, Andrew Dunstan wrote: > On 4/14/20 4:44 PM, Alvaro Herrera wrote: > > On 2020-Apr-14, Andrew Dunstan wrote: > >> One of the things that's a bit sad is that perlcritic doesn't generally > >> let you apply policies to a given set of files or files matching so

Re: cleaning perl code

On 4/14/20 4:44 PM, Alvaro Herrera wrote: > On 2020-Apr-14, Andrew Dunstan wrote: > >> One of the things that's a bit sad is that perlcritic doesn't generally >> let you apply policies to a given set of files or files matching some >> pattern. It would be nice, for instance, to be able to apply so

Re: cleaning perl code

On 2020-Apr-14, Andrew Dunstan wrote: > One of the things that's a bit sad is that perlcritic doesn't generally > let you apply policies to a given set of files or files matching some > pattern. It would be nice, for instance, to be able to apply some > additional standards to strategic library fi

Re: cleaning perl code

On 4/13/20 12:47 PM, Andrew Dunstan wrote: > > OK, I've committed all that stuff. I think that takes care of the > non-controversial part of what I proposed :-) > > OK, it seems there is a majority of people commenting in this thread in favor of not doing more except to reverse the policy of requ

Re: cleaning perl code

On 4/12/20 3:42 AM, Noah Misch wrote: > On Sat, Apr 11, 2020 at 12:13:08PM -0400, Andrew Dunstan wrote: >> --- a/src/tools/msvc/Project.pm >> +++ b/src/tools/msvc/Project.pm >> @@ -420,13 +420,10 @@ sub read_file >> { >> my $filename = shift; >> my $F; >> -my $t = $/; >> - >> -

Re: cleaning perl code

On 4/12/20 6:24 PM, Andrew Dunstan wrote: On 4/12/20 4:12 PM, David Steele wrote: Just in case it is useful, I have attached our old policy file with exceptions and excuses (when we had one). That's a pretty short list for --brutal, well done. I agree there is value in keeping documented the

Re: cleaning perl code

On 4/12/20 4:12 PM, David Steele wrote: > On 4/12/20 3:22 PM, Robert Haas wrote: >> On Sat, Apr 11, 2020 at 11:15 AM Tom Lane wrote: >>> Noah Misch writes: In summary, among those warnings, I see non-negative value in "Code before warnings are enabled" only.  While we're changing

Re: cleaning perl code

On 4/12/20 3:22 PM, Robert Haas wrote: On Sat, Apr 11, 2020 at 11:15 AM Tom Lane wrote: Noah Misch writes: In summary, among those warnings, I see non-negative value in "Code before warnings are enabled" only. While we're changing this, I propose removing Subroutines::RequireFinalReturn. I

Re: cleaning perl code

On Sat, Apr 11, 2020 at 11:15 AM Tom Lane wrote: > Noah Misch writes: > > In summary, among those warnings, I see non-negative value in "Code before > > warnings are enabled" only. While we're changing this, I propose removing > > Subroutines::RequireFinalReturn. > > If it's possible to turn off

Re: cleaning perl code

On Sat, Apr 11, 2020 at 12:13:08PM -0400, Andrew Dunstan wrote: > --- a/src/tools/msvc/Project.pm > +++ b/src/tools/msvc/Project.pm > @@ -420,13 +420,10 @@ sub read_file > { > my $filename = shift; > my $F; > - my $t = $/; > - > - undef $/; > + local $/ = undef; > ope

Re: cleaning perl code

On Sat, Apr 11, 2020 at 11:14:52AM -0400, Tom Lane wrote: > Noah Misch writes: > > In summary, among those warnings, I see non-negative value in "Code before > > warnings are enabled" only. While we're changing this, I propose removing > > Subroutines::RequireFinalReturn. > > If it's possible to

Re: cleaning perl code

Mark Dilger writes: > I'm less concerned with which perlcritic features you enable than I am with > accidentally submitting perl which looks fine to me but breaks the build. I > mostly use perl from within TAP tests, which I run locally before submission > to the project. Can your changes be

Re: cleaning perl code

Andrew Dunstan writes: > On 4/11/20 12:48 PM, Tom Lane wrote: >> Is there a way to modify the test so that it only complains when >> the final return is missing and there are other return(s) with values? >> That would seem like a more narrowly tailored check. > Not AFAICS: >

Re: cleaning perl code

> On Apr 11, 2020, at 9:47 AM, Andrew Dunstan > wrote: > > > On 4/11/20 12:28 PM, Mark Dilger wrote: >> >>> On Apr 11, 2020, at 9:13 AM, Andrew Dunstan >>> wrote: >> Hi Andrew. I appreciate your interest and efforts here. I hope you don't >> mind a few questions/observations about this

Re: cleaning perl code

On 4/11/20 12:48 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 4/11/20 12:30 AM, Noah Misch wrote: >>> In summary, among those warnings, I see non-negative value in "Code before >>> warnings are enabled" only. While we're changing this, I propose removing >>> Subroutines::RequireFinalRetur

Re: cleaning perl code

Andrew Dunstan writes: > On 4/11/20 12:30 AM, Noah Misch wrote: >> In summary, among those warnings, I see non-negative value in "Code before >> warnings are enabled" only. While we're changing this, I propose removing >> Subroutines::RequireFinalReturn. Implicit return values were not a materia

Re: cleaning perl code

On 4/11/20 12:28 PM, Mark Dilger wrote: > >> On Apr 11, 2020, at 9:13 AM, Andrew Dunstan >> wrote: > Hi Andrew. I appreciate your interest and efforts here. I hope you don't > mind a few questions/observations about this effort: Not at all. > >> The >> last one fixes the mixture of high

Re: cleaning perl code

> On Apr 11, 2020, at 9:13 AM, Andrew Dunstan > wrote: Hi Andrew. I appreciate your interest and efforts here. I hope you don't mind a few questions/observations about this effort: > > The > last one fixes the mixture of high and low precedence boolean operators, I did not spot example

Re: cleaning perl code

On 4/11/20 12:30 AM, Noah Misch wrote: > On Thu, Apr 09, 2020 at 11:44:11AM -0400, Andrew Dunstan wrote: >> 39 Always unpack @_ first > Requiring a "my @args = @_" does not improve this code: > > sub CreateSolution > { > ... > if ($visualStudioVersion eq '12.00') > { >

Re: cleaning perl code

Noah Misch writes: > In summary, among those warnings, I see non-negative value in "Code before > warnings are enabled" only. While we're changing this, I propose removing > Subroutines::RequireFinalReturn. If it's possible to turn off just that warning, then +several. It's routinely caused buil

Re: cleaning perl code

On 2020-04-11 06:30, Noah Misch wrote: In summary, among those warnings, I see non-negative value in "Code before warnings are enabled" only. Now that you put it like this, that was also my impression when I first introduced the level 5 warnings and then decided to stop there. -- Peter Eisen

Re: cleaning perl code

On Thu, Apr 09, 2020 at 11:44:11AM -0400, Andrew Dunstan wrote: > 39 Always unpack @_ first Requiring a "my @args = @_" does not improve this code: sub CreateSolution { ... if ($visualStudioVersion eq '12.00') { return new VS2013Solution(@_); }

Re: cleaning perl code

On 4/9/20 2:26 PM, Peter Eisentraut wrote: > On 2020-04-09 19:47, Robert Haas wrote: >> On Thu, Apr 9, 2020 at 11:44 AM Andrew Dunstan >> wrote: >>> We currently only run perlcritic at severity level 5, which is fairly >>> permissive. I'd like to reduce that, ideally to, say, level 3, which is >

Re: cleaning perl code

On 2020-04-09 19:47, Robert Haas wrote: On Thu, Apr 9, 2020 at 11:44 AM Andrew Dunstan wrote: We currently only run perlcritic at severity level 5, which is fairly permissive. I'd like to reduce that, ideally to, say, level 3, which is what I use for the buildfarm code. But let's start by goin

Re: cleaning perl code

On Thu, Apr 9, 2020 at 11:44 AM Andrew Dunstan wrote: > We currently only run perlcritic at severity level 5, which is fairly > permissive. I'd like to reduce that, ideally to, say, level 3, which is > what I use for the buildfarm code. > > But let's start by going to severity level 4. I continue

cleaning perl code

We currently only run perlcritic at severity level 5, which is fairly permissive. I'd like to reduce that, ideally to, say, level 3, which is what I use for the buildfarm code. But let's start by going to severity level 4. Give this perlcriticrc, derived from the buildfarm's: # for policy