Обсуждение: pgsql: Fix precedence problem in new Perl code.

Fix precedence problem in new Perl code.

I think this bit of commit 1f1cd9b5d didn't do quite what I meant :-(

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/59cb323053f4ed582d4e71acaeb5770603f074db

Modified Files
--------------
src/backend/catalog/Catalog.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Mike Blackwell <mike.blackwell@rrd.com> writes:
> In my experience, that would more commonly be written with the lower
> precedence "or" operator (with or without the param list parens):
>  unlink $temp_name or die "unlink: $temp_name: $!";

Yeah, I thought about that, but the pre-existing rename call had ||
and I didn't want to deviate from the existing style; I'm not a good
enough Perl programmer to be entitled to have opinions about Perl style.

Probably all of this code could use a visit from the Perl style police.
I wonder if anyone's tried perlcritic on it recently.

            regards, tom lane

On Fri, May 4, 2018 at 5:28 PM, Mike Blackwell <mike.blackwell@rrd.com> wrote:
> I didn't see a .perlcriticrc file in the project, so ran with our local
> settings.
>
> With those, perlcritic is pretty unhappy, even at -4, though I don't see
> anything that pops out as potentially bug-inducing.  The ones I'd probably
> look fixing at for starters would be the two argument form of open, and
> maybe the .pl files without a #! so perlcritic doesn't mistake them for .pm
> files.
>
> It's also pretty noisy about the possible confusion cause by using a leading
> zero for octal vs oct(), though that's been common practice as far back as
> my memory goes.  Those could be silenced in an rc file if that's preferred.
>
> If there's interest I could put together a patch for some or all of this.
>

(Please don't top-post, it violates our mailcritic policy)

There's been discussion about it before. The consensus was that we
don't care about a good many of the things perlcritic consider to be
very severe, e.g. two-argument open, which is an old and widely used
idiom that I at least have never had any problems with.

Also, I was more than amused yesterday when looking at it against the
buildfarm client code when I got this:

Fatal error while critiquing "./run_build.pl": Not an ARRAY reference
at /usr/share/perl5/vendor_perl/Perl/Critic/Policy/BuiltinFunctions/ProhibitUselessTopic.pm
line 81.

Oh, the irony.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services