Content-Type: multipart/alternative;
 boundary="------------gDpm6SWagPI7PLfqOJeZkSzV"
Message-ID: <5dadbccb-a36c-4195-a4b6-bf5e828fc60d@dunslane.net>
Date: Mon, 16 Sep 2024 08:23:58 -0400
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: pgsql: Perl scripts: eliminate "Useless interpolation" warnings
To: Bruce Momjian <bruce@momjian.us>
Cc: pgsql-committers@lists.postgresql.org
References: <E1spqfS-001Ehn-Cx@gemulon.postgresql.org>
 <66b5b1d6-3f2a-4944-91a3-7e40a565ccac@dunslane.net>
 <ZudAtNRQOkveAa91@momjian.us>
 <e51357f4-4ac5-4b64-b01a-15931485702f@dunslane.net>
 <ZudFaDhhXHg3AAlv@momjian.us>
 <42f2242a-422b-4aa3-8d60-d67b229c4a52@dunslane.net>
 <ZueJ2qppUkTwREIk@momjian.us>
From: Andrew Dunstan <andrew@dunslane.net>
Content-Language: en-US
Autocrypt: addr=andrew@dunslane.net; keydata=
 xsBNBE7KWFkBCAClridxur2AIc7eW2AR7izbfp3EnNefie2HbLF0izW5Ik5UjX2HBXBx4syI
 gY6b0ugohXrr274+baoAlvSbq6cAoQuEVrk5IZFzt20b1Xkx65FwGSEj526yiKLocqkJceSq
 Xr9xcA5SGY+FZv441chh5SU92v4q6z+6LPpoHOh97ptAVXZYNTtU0LevyvD5lja0TzbvJm6C
 eFXitJfnm1pLEr0DGJCR/iUOl/N62Kh4855zZC7NHIjQHPOvV5Stz/l5ilDhvGVk+xkXFPys
 SjZoUr1rXhYLpiyi5sR0X9FHXT0KnGuz1F5ERO7ZTLSSQ6fJwPj6gOk9K+vvoKvoeql5ABEB
 AAHNJEFuZHJldyBEdW5zdGFuIDxhbmRyZXdAZHVuc2xhbmUubmV0PsLAmwQTAQgARQIbAwIX
 gAIZAQULCQgHAgMiAgEGFQoJCAsCBBYCAwECHgcWIQTkPlhGHfx8v0RpFaWZ+n/LWfw7gQUC
 ZFlxxwUJGVGAbgAKCRCZ+n/LWfw7gXikB/9ZdcUy6CTBFIIuL/bVsc1eLEW/gJBjJBF6HxNY
 xgEkAgXAp4Lg4A5U+QB9GouFr7+GYxF0BU4hzoGhNPUWltxnHdMWP8nC/38LAqgMi8L/bbsm
 HW5YPBdWYaAZAPJQVfOAgjTbRUb26KSprpyrrJKW0ZmrZfjhNPcQ72jpWzoPLQqx2X6B0fru
 1jq+cBh8lb6r1mJTim1T3JIn+F/v5VpdQS+EL8xqsHkfzKjIPsW3CIXpkypSk6saA55Rkkbl
 26AW8ftPVB0Q6Lnn6FLt9CP0MGNixBQ55yq8r1K+nCBvCCjvQjM8RDm0UUum0WNl+ifQgTLO
 E8TWEnwVtkBf+3QWzsBNBE7KWFkBCADRnOM0FCzsYW6jtncg+dWIagjUZpvaClmqn/sJluLa
 Q3v1VXMQJzYs3eC1gh386W+XBwLRpDj3jzH81lX+p73Re3d3oJW7X+ffsxuzu5ZVdMUkqBYo
 nkAbKxr6gyJ12F/+JkUVzLcoTN+d/7YsQvUVi7NaKH8mJgjz112O4fUe3p9wfAaFa0RXHc5S
 GPzRTYRRlv/XZBIho4J2tkZOnteZJZ+GbxQVlINt6fd8P6al3MWOvpP/ExJPguEfjOsO6Njy
 xjo3WfpD4lHMOR/Oc3/8mScEF84rF2jXbsFgelWnbPWAvXY+pD0dXOFRkagGmC/viwBDqq5b
 5tk76kKmUbZxABEBAAHCwHwEGAEIACYCGwwWIQTkPlhGHfx8v0RpFaWZ+n/LWfw7gQUCZFlx
 5wUJGVGAjgAKCRCZ+n/LWfw7gf+iB/4g8CPY5jihf5r/8EsoIGe2H+dpVmpPF8YGBzTIvCz/
 fQoOq8AX/pE76QEuFnFZWfjw+wgBXgCVmkox2Eflkk6z4ND3pcwGZ6CfCxTQCDk/dij+2DQ4
 6bmDCy/sBgcbz9mTpoLC11HLoPae6YN9nBNQRZDcEFEu54OaVOqlIdbA6m+POIBCXZdHOFc0
 WoDTgxHRzC1jgQNidyd6tKqcsVJs0dzF0oKTmFFmUAqTdJO12LBuNA1rlqrR3EtpYk8B/wtS
 5dIMD7Q8hwQpL+4C6GNpb6ZKnPkLi47pDOLhz2qBrqN+rqUEsT3YnExYpzj5yOBi+FlmV1Hw
 49QYe1sn2ZPs
In-Reply-To: <ZueJ2qppUkTwREIk@momjian.us>
Archived-At: 
 <https://www.postgresql.org/message-id/5dadbccb-a36c-4195-a4b6-bf5e828fc60d%40dunslane.net>
Precedence: bulk

This is a multi-part message in MIME format.
--------------gDpm6SWagPI7PLfqOJeZkSzV
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


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

--------------gDpm6SWagPI7PLfqOJeZkSzV
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2024-09-15 Su 9:28 PM, Bruce Momjian
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:ZueJ2qppUkTwREIk@momjian.us">
      <pre class="moz-quote-pre" wrap="">On Sun, Sep 15, 2024 at 06:07:21PM -0400, Andrew Dunstan wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">
On 2024-09-15 Su 4:36 PM, Bruce Momjian wrote:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">On Sun, Sep 15, 2024 at 04:33:49PM -0400, Andrew Dunstan wrote:
</pre>
          <blockquote type="cite">
            <pre class="moz-quote-pre" wrap="">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.
</pre>
          </blockquote>
          <pre class="moz-quote-pre" wrap="">I guess it is special only because my policy is more strict so I wanted
my new code to match.  Should I revert it?
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">

Yes I think so.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Okay, done.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">Is the very tiny improvement
not worth the churn in the code?
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">

Yeah, I don't think it is.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
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.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>I would normally write this as</p>
    <p>    my $tempfile = "$filename.tmp";</p>
    <p>or possibly<br>
    </p>
    <p>    my $tempfile = "${filename}.tmp";</p>
    <p>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.<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite" cite="mid:ZueJ2qppUkTwREIk@momjian.us">
      <pre class="moz-quote-pre" wrap="">

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.
</pre>
    </blockquote>
    <p><br>
    </p>
    <p><span style="white-space: pre-wrap">Sure, or for something in between, there's the buildfarm policy at <a class="moz-txt-link-rfc2396E" href="https://github.com/PGBuildFarm/client-code/blob/main/.perlcriticrc">&lt;https://github.com/PGBuildFarm/client-code/blob/main/.perlcriticrc&gt;</a></span></p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <p><span style="white-space: pre-wrap">cheers</span></p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <p><span style="white-space: pre-wrap">andrew
</span></p>
    <pre class="moz-signature"
    signature-switch-id="d0437855-2267-4610-80b3-83167ec45b0b" cols="72">--
Andrew Dunstan
EDB: <a class="moz-txt-link-freetext" href="https://www.enterprisedb.com">https://www.enterprisedb.com</a>
</pre>
  </body>
</html>

--------------gDpm6SWagPI7PLfqOJeZkSzV--