Re: [COMMITTERS] pgsql: Clean up Perl code according to perlcritic - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [COMMITTERS] pgsql: Clean up Perl code according to perlcritic
Date
Msg-id 4193.1490662733@sss.pgh.pa.us
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Clean up Perl code according to perlcritic  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [COMMITTERS] pgsql: Clean up Perl code according to perlcritic  (ilmari@ilmari.org (Dagfinn Ilmari Mannsåker))
Re: [COMMITTERS] pgsql: Clean up Perl code according toperlcritic  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
List pgsql-hackers
I wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> Clean up Perl code according to perlcritic

> This seems to have broken the regression tests (specifically, dblink)
> on at least some of the Windows buildfarm critters.

I'm hardly a Perl expert, but it looks to me like the culprit is this
hunk in vcregress.pl:

@@ -521,8 +521,9 @@ sub fetchRegressOpts        # an unhandled variable reference.  Ignore anything that isn't an
# option starting with "--".        @opts = grep {
 
-            s/\Q$(top_builddir)\E/\"$topdir\"/;
-            $_ !~ /\$\(/ && $_ =~ /^--/
+            my $x = $_;
+            $x =~ s/\Q$(top_builddir)\E/\"$topdir\"/;
+            $x !~ /\$\(/ && $x =~ /^--/        } split(/\s+/, $1);    }    if ($m =~ /^\s*ENCODING\s*=\s*(\S+)/m)

The first line in that block is actually intending to modify the value
it's inspecting, and perlcritic's "improved" version carefully removes
the side-effect.

No doubt there are cleaner ways to do that (the comments in "man perlfunc"
about this coding technique are not positive), but this way is not
cleaner, it is broken.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Re: Bug in get_partition_for_tuple
Next
From: Amit Langote
Date:
Subject: Re: Partitioned tables and relfilenode