pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] perlcritic: prohibit map and grep in void context
15+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2021-08-31 bb466c6b09 Prohibit map and grep in void context
* [PATCH] perlcritic: prohibit map and grep in void context
@ 2021-07-27 15:51 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 0 replies; 15+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2021-07-27 15:51 UTC (permalink / raw)
---
contrib/intarray/bench/create_test.pl | 2 +-
src/tools/perlcheck/perlcriticrc | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
inline patch 1: contrib/intarray/bench/create_test.pl, src/tools/perlcheck/perlcriticrc (+8/-1 lines) download inline-1.patch
diff --git a/contrib/intarray/bench/create_test.pl b/contrib/intarray/bench/create_test.pl
index 993a4572f4..ae8d72bab0 100755
--- a/contrib/intarray/bench/create_test.pl
+++ b/contrib/intarray/bench/create_test.pl
@@ -51,7 +51,7 @@
else
{
print $msg "$i\t{" . join(',', @sect) . "}\n";
- map { print $map "$i\t$_\n" } @sect;
+ print $map "$i\t$_\n" foreach @sect;
}
}
close $map;
diff --git a/src/tools/perlcheck/perlcriticrc b/src/tools/perlcheck/perlcriticrc
index e230111b23..9267fb43b2 100644
--- a/src/tools/perlcheck/perlcriticrc
+++ b/src/tools/perlcheck/perlcriticrc
@@ -22,3 +22,10 @@ verbose = %f: %m at line %l, column %c. %e. ([%p] Severity: %s)\n
# insist on use of the warnings pragma
[TestingAndDebugging::RequireUseWarnings]
severity = 5
+
+# forbid grep and map in void context
+[BuiltinFunctions::ProhibitVoidGrep]
+severity = 5
+
+[BuiltinFunctions::ProhibitVoidMap]
+severity = 5
--
2.30.2
* perlcritic: prohibit map and grep in void conext
@ 2021-07-27 16:06 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 3 replies; 15+ messages in thread
From: ilmari@ilmari.org @ 2021-07-27 16:06 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hi hackers,
In the patches for improving the MSVC build process, I noticed a use of
`map` in void context. This is considered bad form, and has a
perlcritic policy forbidding it:
https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitVoidMap.
Attached is a patch that increases severity of that and the
corresponding `grep` policy to 5 to enable it in our perlcritic policy,
and fixes the one use that had already snuck in.
- ilmari
Attachments:
0001-perlcritic-prohibit-map-and-grep-in-void-context.patch (text/x-diff, 1.3K) download
From c82b08ce047ab47188753806984f1dc5be365556 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 27 Jul 2021 16:51:29 +0100
Subject: [PATCH] perlcritic: prohibit map and grep in void context
---
contrib/intarray/bench/create_test.pl | 2 +-
src/tools/perlcheck/perlcriticrc | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/contrib/intarray/bench/create_test.pl b/contrib/intarray/bench/create_test.pl
index 993a4572f4..ae8d72bab0 100755
--- a/contrib/intarray/bench/create_test.pl
+++ b/contrib/intarray/bench/create_test.pl
@@ -51,7 +51,7 @@
else
{
print $msg "$i\t{" . join(',', @sect) . "}\n";
- map { print $map "$i\t$_\n" } @sect;
+ print $map "$i\t$_\n" foreach @sect;
}
}
close $map;
diff --git a/src/tools/perlcheck/perlcriticrc b/src/tools/perlcheck/perlcriticrc
index e230111b23..9267fb43b2 100644
--- a/src/tools/perlcheck/perlcriticrc
+++ b/src/tools/perlcheck/perlcriticrc
@@ -22,3 +22,10 @@ verbose = %f: %m at line %l, column %c. %e. ([%p] Severity: %s)\n
# insist on use of the warnings pragma
[TestingAndDebugging::RequireUseWarnings]
severity = 5
+
+# forbid grep and map in void context
+[BuiltinFunctions::ProhibitVoidGrep]
+severity = 5
+
+[BuiltinFunctions::ProhibitVoidMap]
+severity = 5
--
2.30.2
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-07-27 19:09 Daniel Gustafsson <daniel@yesql.se>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2 siblings, 1 reply; 15+ messages in thread
From: Daniel Gustafsson @ 2021-07-27 19:09 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers@lists.postgresql.org
> On 27 Jul 2021, at 18:06, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
> Attached is a patch that increases severity of that and the
> corresponding `grep` policy to 5 to enable it in our perlcritic policy,
> and fixes the one use that had already snuck in.
+1, the use of foreach also improves readability a fair bit IMO.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-07-28 04:23 Michael Paquier <michael@paquier.xyz>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 0 replies; 15+ messages in thread
From: Michael Paquier @ 2021-07-28 04:23 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers@lists.postgresql.org
On Tue, Jul 27, 2021 at 09:09:10PM +0200, Daniel Gustafsson wrote:
> On 27 Jul 2021, at 18:06, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>> Attached is a patch that increases severity of that and the
>> corresponding `grep` policy to 5 to enable it in our perlcritic policy,
>> and fixes the one use that had already snuck in.
>
> +1, the use of foreach also improves readability a fair bit IMO.
Sounds interesting to avoid. pgperlcritic does not complain here
after this patch.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-07-28 11:10 Andrew Dunstan <andrew@dunslane.net>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2 siblings, 1 reply; 15+ messages in thread
From: Andrew Dunstan @ 2021-07-28 11:10 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers@lists.postgresql.org
On 7/27/21 12:06 PM, Dagfinn Ilmari Mannsåker wrote:
> Hi hackers,
>
> In the patches for improving the MSVC build process, I noticed a use of
> `map` in void context. This is considered bad form, and has a
> perlcritic policy forbidding it:
> https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitVoidMap.
>
> Attached is a patch that increases severity of that and the
> corresponding `grep` policy to 5 to enable it in our perlcritic policy,
> and fixes the one use that had already snuck in.
>
Personally I'm OK with it, but previous attempts to enforce perlcritic
policies have met with a less than warm reception, and we had to back
off. Maybe this one will fare better.
I keep the buildfarm code perlcritic compliant down to severity 3 with a
handful of exceptions.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-07-28 11:26 Daniel Gustafsson <daniel@yesql.se>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Gustafsson @ 2021-07-28 11:26 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers@lists.postgresql.org
> On 28 Jul 2021, at 13:10, Andrew Dunstan <andrew@dunslane.net> wrote:
>
> On 7/27/21 12:06 PM, Dagfinn Ilmari Mannsåker wrote:
>> Hi hackers,
>>
>> In the patches for improving the MSVC build process, I noticed a use of
>> `map` in void context. This is considered bad form, and has a
>> perlcritic policy forbidding it:
>> https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitVoidMap.
>>
>> Attached is a patch that increases severity of that and the
>> corresponding `grep` policy to 5 to enable it in our perlcritic policy,
>> and fixes the one use that had already snuck in.
>
> Personally I'm OK with it, but previous attempts to enforce perlcritic
> policies have met with a less than warm reception, and we had to back
> off. Maybe this one will fare better.
I'm fine with increasing this policy, but I don't have strong feelings. If we
feel the perlcritic policy change is too much, I would still prefer to go ahead
with the map rewrite part of the patch though.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-07 23:15 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2 siblings, 0 replies; 15+ messages in thread
From: ilmari@ilmari.org @ 2021-08-07 23:15 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
> Hi hackers,
>
> In the patches for improving the MSVC build process, I noticed a use of
> `map` in void context. This is considered bad form, and has a
> perlcritic policy forbidding it:
> https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitVoidMap.
>
> Attached is a patch that increases severity of that and the
> corresponding `grep` policy to 5 to enable it in our perlcritic policy,
> and fixes the one use that had already snuck in.
Added to the 2021-09 commitfest: https://commitfest.postgresql.org/34/3278/
- ilmari
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-27 06:10 Michael Paquier <michael@paquier.xyz>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 2 replies; 15+ messages in thread
From: Michael Paquier @ 2021-08-27 06:10 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers@lists.postgresql.org
On Wed, Jul 28, 2021 at 01:26:23PM +0200, Daniel Gustafsson wrote:
> I'm fine with increasing this policy, but I don't have strong feelings. If we
> feel the perlcritic policy change is too much, I would still prefer to go ahead
> with the map rewrite part of the patch though.
I have no issue either about the rewrite part of the patch, so I'd
tend to just do this part and move on. Daniel, would you like to
apply that?
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-27 07:15 Daniel Gustafsson <daniel@yesql.se>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 15+ messages in thread
From: Daniel Gustafsson @ 2021-08-27 07:15 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; pgsql-hackers@lists.postgresql.org
> On 27 Aug 2021, at 08:10, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jul 28, 2021 at 01:26:23PM +0200, Daniel Gustafsson wrote:
>> I'm fine with increasing this policy, but I don't have strong feelings. If we
>> feel the perlcritic policy change is too much, I would still prefer to go ahead
>> with the map rewrite part of the patch though.
>
> I have no issue either about the rewrite part of the patch, so I'd
> tend to just do this part and move on. Daniel, would you like to
> apply that?
Sure, I can take care of that.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-27 10:32 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 15+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2021-08-27 10:32 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Daniel Gustafsson <daniel@yesql.se>; Andrew Dunstan <andrew@dunslane.net>; pgsql-hackers@lists.postgresql.org
Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Jul 28, 2021 at 01:26:23PM +0200, Daniel Gustafsson wrote:
>> I'm fine with increasing this policy, but I don't have strong feelings. If we
>> feel the perlcritic policy change is too much, I would still prefer to go ahead
>> with the map rewrite part of the patch though.
>
> I have no issue either about the rewrite part of the patch, so I'd
> tend to just do this part and move on. Daniel, would you like to
> apply that?
Why the resistance to the perlcritic part? That one case is the only
violation in the tree today, and it's a pattern we don't want to let
back in (I will surely object every time I see it when reviewing
patches), so why not automate it?
- ilmari
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-30 18:27 Andrew Dunstan <andrew@dunslane.net>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Dunstan @ 2021-08-30 18:27 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; Michael Paquier <michael@paquier.xyz>; +Cc: Daniel Gustafsson <daniel@yesql.se>; pgsql-hackers@lists.postgresql.org
On 8/27/21 6:32 AM, Dagfinn Ilmari Mannsåker wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>
>> On Wed, Jul 28, 2021 at 01:26:23PM +0200, Daniel Gustafsson wrote:
>>> I'm fine with increasing this policy, but I don't have strong feelings. If we
>>> feel the perlcritic policy change is too much, I would still prefer to go ahead
>>> with the map rewrite part of the patch though.
>> I have no issue either about the rewrite part of the patch, so I'd
>> tend to just do this part and move on. Daniel, would you like to
>> apply that?
> Why the resistance to the perlcritic part? That one case is the only
> violation in the tree today, and it's a pattern we don't want to let
> back in (I will surely object every time I see it when reviewing
> patches), so why not automate it?
>
There doesn't seem to have been much pushback, so let's try it and see.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-31 01:22 Michael Paquier <michael@paquier.xyz>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2021-08-31 01:22 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; Daniel Gustafsson <daniel@yesql.se>; pgsql-hackers@lists.postgresql.org
On Mon, Aug 30, 2021 at 02:27:09PM -0400, Andrew Dunstan wrote:
> There doesn't seem to have been much pushback, so let's try it and see.
Okay, fine by me.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-31 04:19 Julien Rouhaud <rjuju123@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 15+ messages in thread
From: Julien Rouhaud @ 2021-08-31 04:19 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrew Dunstan <andrew@dunslane.net>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Aug 31, 2021 at 9:23 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Aug 30, 2021 at 02:27:09PM -0400, Andrew Dunstan wrote:
> > There doesn't seem to have been much pushback, so let's try it and see.
>
> Okay, fine by me.
+1
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-31 09:30 Daniel Gustafsson <daniel@yesql.se>
parent: Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Gustafsson @ 2021-08-31 09:30 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Andrew Dunstan <andrew@dunslane.net>; Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
> On 31 Aug 2021, at 06:19, Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Tue, Aug 31, 2021 at 9:23 AM Michael Paquier <michael@paquier.xyz> wrote:
>>
>> On Mon, Aug 30, 2021 at 02:27:09PM -0400, Andrew Dunstan wrote:
>>> There doesn't seem to have been much pushback, so let's try it and see.
>>
>> Okay, fine by me.
>
> +1
Since there is concensus in the thread with no -1’s, I’ve pushed this to master
now.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: perlcritic: prohibit map and grep in void conext
@ 2021-08-31 09:55 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 0 replies; 15+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2021-08-31 09:55 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; Julien Rouhaud <rjuju123@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Andrew Dunstan <andrew@dunslane.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, 31 Aug 2021, at 10:30, Daniel Gustafsson wrote:
> > On 31 Aug 2021, at 06:19, Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > On Tue, Aug 31, 2021 at 9:23 AM Michael Paquier <michael@paquier.xyz> wrote:
> >>
> >> On Mon, Aug 30, 2021 at 02:27:09PM -0400, Andrew Dunstan wrote:
> >>> There doesn't seem to have been much pushback, so let's try it and see.
> >>
> >> Okay, fine by me.
> >
> > +1
>
> Since there is concensus in the thread with no -1’s, I’ve pushed this to master
> now.
Thanks!
- ilmari
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2021-08-31 09:55 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 15:51 [PATCH] perlcritic: prohibit map and grep in void context Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-07-27 16:06 perlcritic: prohibit map and grep in void conext Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-07-27 19:09 ` Re: perlcritic: prohibit map and grep in void conext Daniel Gustafsson <daniel@yesql.se>
2021-07-28 04:23 ` Re: perlcritic: prohibit map and grep in void conext Michael Paquier <michael@paquier.xyz>
2021-07-28 11:10 ` Re: perlcritic: prohibit map and grep in void conext Andrew Dunstan <andrew@dunslane.net>
2021-07-28 11:26 ` Re: perlcritic: prohibit map and grep in void conext Daniel Gustafsson <daniel@yesql.se>
2021-08-27 06:10 ` Re: perlcritic: prohibit map and grep in void conext Michael Paquier <michael@paquier.xyz>
2021-08-27 07:15 ` Re: perlcritic: prohibit map and grep in void conext Daniel Gustafsson <daniel@yesql.se>
2021-08-27 10:32 ` Re: perlcritic: prohibit map and grep in void conext Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-08-30 18:27 ` Re: perlcritic: prohibit map and grep in void conext Andrew Dunstan <andrew@dunslane.net>
2021-08-31 01:22 ` Re: perlcritic: prohibit map and grep in void conext Michael Paquier <michael@paquier.xyz>
2021-08-31 04:19 ` Re: perlcritic: prohibit map and grep in void conext Julien Rouhaud <rjuju123@gmail.com>
2021-08-31 09:30 ` Re: perlcritic: prohibit map and grep in void conext Daniel Gustafsson <daniel@yesql.se>
2021-08-31 09:55 ` Re: perlcritic: prohibit map and grep in void conext Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2021-08-07 23:15 ` Re: perlcritic: prohibit map and grep in void conext Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox