* [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist @ 2020-06-11 6:25 SeongJae Park 2020-06-11 6:25 ` [PATCH v4 1/2] checkpatch: support deprecated terms checking SeongJae Park ` (2 more replies) 0 siblings, 3 replies; 35+ messages in thread From: SeongJae Park @ 2020-06-11 6:25 UTC (permalink / raw) To: akpm, apw, joe; +Cc: SeongJae Park, colin.king, sj38.park, jslaby, linux-kernel From: SeongJae Park <sjpark@amazon.de> This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' and 2) set the 'blacklist' and 'whitelist' as deprecated with replacement suggestion of 'denylist' and 'allowlist', because the suggestions are incontrovertible, doesn't make people hurt, and more self-explanatory. The patches are based on latest 'next/master'. You can get the complete git tree at: https://github.com/sjp38/linux/tree/patches/checkpatch/deprecate_blacklist_whitelist_on_next_v4 Patch History ============= Changes from v3 (https://lore.kernel.org/lkml/20200610065223.29894-1-sjpark@amazon.com/) - Reuse the file read code for 'spelling.txt' (Joe Perches) - Suggest 'denylist' rather than 'blocklist' (Jiri Slaby) - Rebased on today's next/master Changes from v2 (https://lore.kernel.org/lkml/20200609122549.26304-1-sjpark@amazon.com/) - Implement and use deprecated terms check Changes from v1 (https://lore.kernel.org/lkml/20200609121843.24147-1-sjpark@amazon.com/) - Remove unnecessary commit message SeongJae Park (2): checkpatch: support deprecated terms checking scripts/deprecated_terms: Recommend denylist/allowlist instead of blacklist/whitelist scripts/checkpatch.pl | 60 +++++++++++++++++++++++++++--------- scripts/deprecated_terms.txt | 7 +++++ 2 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 scripts/deprecated_terms.txt -- 2.17.1 ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v4 1/2] checkpatch: support deprecated terms checking 2020-06-11 6:25 [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park @ 2020-06-11 6:25 ` SeongJae Park 2020-07-25 13:02 ` Michał Mirosław 2020-06-11 6:25 ` [PATCH v4 2/2] scripts/deprecated_terms: Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park 2020-06-11 6:35 ` [PATCH v4 0/2] " Joe Perches 2 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-06-11 6:25 UTC (permalink / raw) To: akpm, apw, joe; +Cc: SeongJae Park, colin.king, sj38.park, jslaby, linux-kernel From: SeongJae Park <sjpark@amazon.de> Some terms could be deprecated for various reasons, but it is hard to update the entire old usages. That said, we could at least encourage new patches to use the suggested replacements. This commit adds check of deprecated terms in the 'checkpatch.pl' for that. The script will get deprecated terms and suggested replacements of those from 'scripts/deprecated_terms.txt' file and warn if the deprecated terms are used. The mechanism and the format of the file are almost the same as that of 'spelling.txt'. For the reason, this commit modularizes the read of the 'spelling.txt' and reuses. Signed-off-by: SeongJae Park <sjpark@amazon.de> --- scripts/checkpatch.pl | 60 +++++++++++++++++++++++++++--------- scripts/deprecated_terms.txt | 5 +++ 2 files changed, 50 insertions(+), 15 deletions(-) create mode 100644 scripts/deprecated_terms.txt diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 524df88f9364..c672091932bb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -57,6 +57,7 @@ my $max_line_length = 100; my $ignore_perl_version = 0; my $minimum_perl_version = 5.10.0; my $min_conf_desc_length = 4; +my $deprecated_terms_file = "$D/deprecated_terms.txt"; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; @@ -692,29 +693,40 @@ our $allowed_asm_includes = qr{(?x: )}; # memory.h: ARM has a custom one -# Load common spelling mistakes and build regular expression list. -my $misspellings; -my %spelling_fix; +sub read_word_corrections { + my ($file, $fixesRef) = @_; + my $suspects; -if (open(my $spelling, '<', $spelling_file)) { - while (<$spelling>) { - my $line = $_; + if (open(my $corrections, '<', $file)) { + while (<$corrections>) { + my $line = $_; - $line =~ s/\s*\n?$//g; - $line =~ s/^\s*//g; + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; - next if ($line =~ m/^\s*#/); - next if ($line =~ m/^\s*$/); + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); - my ($suspect, $fix) = split(/\|\|/, $line); + my ($suspect, $fix) = split(/\|\|/, $line); - $spelling_fix{$suspect} = $fix; + $fixesRef->{$suspect} = $fix; + } + close($corrections); + } else { + warn "No correction will be found - file '$file': $!\n"; } - close($spelling); -} else { - warn "No typos will be found - file '$spelling_file': $!\n"; } +# Load deprecated terms and build regular expression list. +my %deprecated_terms_fix; +read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix); +my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix; + +# Load common spelling mistakes and build regular expression list. +my $misspellings; +my %spelling_fix; +read_word_corrections($spelling_file, \%spelling_fix); + if ($codespell) { if (open(my $spelling, '<', $codespellfile)) { while (<$spelling>) { @@ -2957,6 +2969,24 @@ sub process { } } +# Check for deprecated terms + if (defined($deprecated_terms) && + ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { + while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { + my $deprecated_term = $1; + my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; + $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); + $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); + my $msg_level = \&WARN; + $msg_level = \&CHK if ($file); + if (&{$msg_level}("DEPRECATED_TERM", + "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; + } + } + } + # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt new file mode 100644 index 000000000000..6faa06451c3d --- /dev/null +++ b/scripts/deprecated_terms.txt @@ -0,0 +1,5 @@ +# License: GPLv2 +# +# The format of each line is: +# deprecated||suggested +# -- 2.17.1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-06-11 6:25 ` [PATCH v4 1/2] checkpatch: support deprecated terms checking SeongJae Park @ 2020-07-25 13:02 ` Michał Mirosław 2020-07-25 16:36 ` Joe Perches 2020-07-25 17:29 ` Joe Perches 0 siblings, 2 replies; 35+ messages in thread From: Michał Mirosław @ 2020-07-25 13:02 UTC (permalink / raw) To: SeongJae Park Cc: linux-kernel, akpm, apw, joe, colin.king, sj38.park, jslaby, pavel Hello, I see that this patch went into next and is already inciting people to do wrong things [1]. Can you please fix it to require '--subjective' switch or otherwise mark it clearly as suggestion-only? The coding-style as in Linus' master says about *NEW* uses of the words listed (those introductions I expect to be actually rare) and not about existing use in the code or industry. Making a noise about all uses found surely will generate a lot more irrelevant patches. [1] https://www.spinics.net/lists/linux-tegra/msg51849.html Best Regards Michał Mirosław ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-25 13:02 ` Michał Mirosław @ 2020-07-25 16:36 ` Joe Perches 2020-07-25 17:29 ` Joe Perches 1 sibling, 0 replies; 35+ messages in thread From: Joe Perches @ 2020-07-25 16:36 UTC (permalink / raw) To: Michał Mirosław, SeongJae Park, Andrew Morton Cc: linux-kernel, akpm, apw, colin.king, sj38.park, jslaby, pavel On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > Hello, > > I see that this patch went into next and is already inciting people to > do wrong things [1]. Can you please fix it to require '--subjective' > switch or otherwise mark it clearly as suggestion-only? > > The coding-style as in Linus' master says about *NEW* uses of the words > listed (those introductions I expect to be actually rare) and not about > existing use in the code or industry. Making a noise about all uses > found surely will generate a lot more irrelevant patches. > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html I was never a big fan of this change. Andrew, can you revert this please? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-25 13:02 ` Michał Mirosław 2020-07-25 16:36 ` Joe Perches @ 2020-07-25 17:29 ` Joe Perches 2020-07-25 23:35 ` SeongJae Park 1 sibling, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-07-25 17:29 UTC (permalink / raw) To: Michał Mirosław, SeongJae Park, Andrew Morton Cc: linux-kernel, akpm, apw, colin.king, sj38.park, jslaby, pavel On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > Hello, > > I see that this patch went into next and is already inciting people to > do wrong things [1]. Can you please fix it to require '--subjective' > switch or otherwise mark it clearly as suggestion-only? > > The coding-style as in Linus' master says about *NEW* uses of the words > listed (those introductions I expect to be actually rare) and not about > existing use in the code or industry. Making a noise about all uses > found surely will generate a lot more irrelevant patches. > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html And if not reverted, perhaps do not check existing files at all but only check patches and change the message to show only suggestions not from a specification. --- scripts/checkpatch.pl | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e9fde28eb0de..7ef1ba80cb20 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2970,21 +2970,16 @@ sub process { } } -# Check for deprecated terms - if (defined($deprecated_terms) && +# Check for deprecated terms not used by a specification (not used on files) + if (!$file && defined($deprecated_terms) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { - my $deprecated_term = $1; - my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; - $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); - $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); - my $msg_level = \&WARN; - $msg_level = \&CHK if ($file); - if (&{$msg_level}("DEPRECATED_TERM", - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; - } + my $deprecate = $1; + my $suggest = $deprecated_terms_fix{lc($deprecate)}; + $suggest = ucfirst($suggest) if ($deprecate =~ /^[A-Z]/); + $suggest = uc($suggest) if ($deprecate =~ /^[A-Z]+$/); + CHK("DEPRECATED_TERM", + "Use of '$deprecate' is controversial - if not required by specification, perhaps '$suggest' instead\n" . $herecurr); } } ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: Re: checkpatch: support deprecated terms checking 2020-07-25 17:29 ` Joe Perches @ 2020-07-25 23:35 ` SeongJae Park 2020-07-26 4:27 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-07-25 23:35 UTC (permalink / raw) To: Joe Perches Cc: Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, sj38.park, jslaby, pavel, SeongJae Park On Sat, 25 Jul 2020 10:29:23 -0700 Joe Perches <joe@perches.com> wrote: > On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > > Hello, > > > > I see that this patch went into next and is already inciting people to > > do wrong things [1]. Can you please fix it to require '--subjective' > > switch or otherwise mark it clearly as suggestion-only? > > > > The coding-style as in Linus' master says about *NEW* uses of the words > > listed (those introductions I expect to be actually rare) and not about > > existing use in the code or industry. Making a noise about all uses > > found surely will generate a lot more irrelevant patches. > > > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html > > And if not reverted, perhaps do not check existing files > at all but only check patches and change the message to > show only suggestions not from a specification. Agreed for this case. However, excluding existing file check doesn't fully avoid this problem. Also, more terms having different deprecation rules might be added in future. How about allowing file check but show reference in the suggestion message as below? > --- [...] Thanks, SeongJae Park ================================ >8 =========================================== From aeb852296bc40ca1de8a6a11f4d5368b02d2e417 Mon Sep 17 00:00:00 2001 From: SeongJae Park <sjpark@amazon.de> Date: Sun, 26 Jul 2020 01:14:48 +0200 Subject: [PATCH] scripts/deprecatd_terms: provide references Deprecation of terms could have special rules. For example, 'slave' is ok for existing usages. Same to 'master', but it's also ok unless it's used with 'slave'. This commit provides the references for such rules. Signed-off-by: SeongJae Park <sjpark@amazon.de> --- scripts/checkpatch.pl | 2 +- scripts/deprecated_terms.txt | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e9fde28eb0de..77f5f777b053 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2981,7 +2981,7 @@ sub process { my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("DEPRECATED_TERM", - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && + "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead. See: scripts/deprecated_terms.txt\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; } diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt index 1be27a24187b..d92b9c896fce 100644 --- a/scripts/deprecated_terms.txt +++ b/scripts/deprecated_terms.txt @@ -3,8 +3,10 @@ # The format of each line is: # deprecated||suggested # +# If special rules are applied on the terms, please comment those. +# +# Refer to "4) Naming" section of Documentation/process/coding-style.rst for +# below three terms. blacklist||(denylist|blocklist) -# For other alternatives of 'slave', Please refer to -# Documentation/process/coding-style.rst slave||(secondary|target|...) whitelist||(allowlist|passlist) -- 2.17.1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: Re: checkpatch: support deprecated terms checking 2020-07-25 23:35 ` SeongJae Park @ 2020-07-26 4:27 ` Joe Perches 2020-07-26 7:18 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-07-26 4:27 UTC (permalink / raw) To: SeongJae Park Cc: Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 2020-07-26 at 01:35 +0200, SeongJae Park wrote: > On Sat, 25 Jul 2020 10:29:23 -0700 Joe Perches <joe@perches.com> wrote: > > > On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > > > Hello, > > > > > > I see that this patch went into next and is already inciting people to > > > do wrong things [1]. Can you please fix it to require '--subjective' > > > switch or otherwise mark it clearly as suggestion-only? > > > > > > The coding-style as in Linus' master says about *NEW* uses of the words > > > listed (those introductions I expect to be actually rare) and not about > > > existing use in the code or industry. Making a noise about all uses > > > found surely will generate a lot more irrelevant patches. > > > > > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html > > > > And if not reverted, perhaps do not check existing files > > at all but only check patches and change the message to > > show only suggestions not from a specification. > > Agreed for this case. However, excluding existing file check doesn't fully > avoid this problem. Also, more terms having different deprecation rules might > be added in future. How about allowing file check but show reference in the > suggestion message as below? The general problem is that drivers/staging, net/ and drivers/net all have --strict on by default. Emitting these deprecated terms messages with -f --file uses for files in those directories isn't a great idea. > diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt [] > @@ -3,8 +3,10 @@ > # The format of each line is: > # deprecated||suggested > # > +# If special rules are applied on the terms, please comment those. Disagree. Comments about these existing uses aren't helpful. > +# > +# Refer to "4) Naming" section of Documentation/process/coding-style.rst for > +# below three terms. > blacklist||(denylist|blocklist) > -# For other alternatives of 'slave', Please refer to > -# Documentation/process/coding-style.rst > slave||(secondary|target|...) > whitelist||(allowlist|passlist) ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 4:27 ` Joe Perches @ 2020-07-26 7:18 ` SeongJae Park 2020-07-26 7:29 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-07-26 7:18 UTC (permalink / raw) To: Joe Perches Cc: SeongJae Park, Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sat, 25 Jul 2020 21:27:07 -0700 Joe Perches <joe@perches.com> wrote: > On Sun, 2020-07-26 at 01:35 +0200, SeongJae Park wrote: > > On Sat, 25 Jul 2020 10:29:23 -0700 Joe Perches <joe@perches.com> wrote: > > > > > On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > > > > Hello, > > > > > > > > I see that this patch went into next and is already inciting people to > > > > do wrong things [1]. Can you please fix it to require '--subjective' > > > > switch or otherwise mark it clearly as suggestion-only? > > > > > > > > The coding-style as in Linus' master says about *NEW* uses of the words > > > > listed (those introductions I expect to be actually rare) and not about > > > > existing use in the code or industry. Making a noise about all uses > > > > found surely will generate a lot more irrelevant patches. > > > > > > > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html > > > > > > And if not reverted, perhaps do not check existing files > > > at all but only check patches and change the message to > > > show only suggestions not from a specification. > > > > Agreed for this case. However, excluding existing file check doesn't fully > > avoid this problem. Also, more terms having different deprecation rules might > > be added in future. How about allowing file check but show reference in the > > suggestion message as below? > > The general problem is that drivers/staging, net/ and drivers/net > all have --strict on by default. > > Emitting these deprecated terms messages with -f --file uses for > files in those directories isn't a great idea. Thank you for kindly explaining your concenrs in detail. However, I think it's ok to do this check even without '--strict' for files if we explicitly says it's suggestion only, as Michal said. My patch does so. > > > diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt > [] > > @@ -3,8 +3,10 @@ > > # The format of each line is: > > # deprecated||suggested > > # > > +# If special rules are applied on the terms, please comment those. > > Disagree. Comments about these existing uses aren't helpful. Sorry, I don't understand your point here. Why do you think it's not helpful? If 'checkpatch' finds the deprecated terms, it will ask people to read this file, which explains special rules for each of the deprecations if exists. The rule is, in the case of 'slave', 'applies to new uses only'. Therefore, people could stop sending the noisy unnecessary patches to the maintainers. Thanks, SeongJae Park > > > +# > > +# Refer to "4) Naming" section of Documentation/process/coding-style.rst for > > +# below three terms. > > blacklist||(denylist|blocklist) > > -# For other alternatives of 'slave', Please refer to > > -# Documentation/process/coding-style.rst > > slave||(secondary|target|...) > > whitelist||(allowlist|passlist) > ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 7:18 ` SeongJae Park @ 2020-07-26 7:29 ` Joe Perches 2020-07-26 7:45 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-07-26 7:29 UTC (permalink / raw) To: SeongJae Park Cc: Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 2020-07-26 at 09:18 +0200, SeongJae Park wrote: > On Sat, 25 Jul 2020 21:27:07 -0700 Joe Perches <joe@perches.com> wrote: > > > On Sun, 2020-07-26 at 01:35 +0200, SeongJae Park wrote: > > > On Sat, 25 Jul 2020 10:29:23 -0700 Joe Perches <joe@perches.com> wrote: > > > > > > > On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > > > > > Hello, > > > > > > > > > > I see that this patch went into next and is already inciting people to > > > > > do wrong things [1]. Can you please fix it to require '--subjective' > > > > > switch or otherwise mark it clearly as suggestion-only? > > > > > > > > > > The coding-style as in Linus' master says about *NEW* uses of the words > > > > > listed (those introductions I expect to be actually rare) and not about > > > > > existing use in the code or industry. Making a noise about all uses > > > > > found surely will generate a lot more irrelevant patches. > > > > > > > > > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html > > > > > > > > And if not reverted, perhaps do not check existing files > > > > at all but only check patches and change the message to > > > > show only suggestions not from a specification. > > > > > > Agreed for this case. However, excluding existing file check doesn't fully > > > avoid this problem. Also, more terms having different deprecation rules might > > > be added in future. How about allowing file check but show reference in the > > > suggestion message as below? > > > > The general problem is that drivers/staging, net/ and drivers/net > > all have --strict on by default. > > > > Emitting these deprecated terms messages with -f --file uses for > > files in those directories isn't a great idea. > > Thank you for kindly explaining your concenrs in detail. However, I think it's > ok to do this check even without '--strict' for files if we explicitly says > it's suggestion only, as Michal said. My patch does so. > > > > diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt > > [] > > > @@ -3,8 +3,10 @@ > > > # The format of each line is: > > > # deprecated||suggested > > > # > > > +# If special rules are applied on the terms, please comment those. > > > > Disagree. Comments about these existing uses aren't helpful. > > Sorry, I don't understand your point here. Why do you think it's not helpful? > If 'checkpatch' finds the deprecated terms, it will ask people to read this > file, which explains special rules for each of the deprecations if exists. The > rule is, in the case of 'slave', 'applies to new uses only'. Therefore, people > could stop sending the noisy unnecessary patches to the maintainers. Because it will describe this for _every_ instance of any deprecated word in the file. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 7:29 ` Joe Perches @ 2020-07-26 7:45 ` SeongJae Park 2020-07-26 14:50 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-07-26 7:45 UTC (permalink / raw) To: Joe Perches Cc: SeongJae Park, Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 26 Jul 2020 00:29:05 -0700 Joe Perches <joe@perches.com> wrote: > On Sun, 2020-07-26 at 09:18 +0200, SeongJae Park wrote: > > On Sat, 25 Jul 2020 21:27:07 -0700 Joe Perches <joe@perches.com> wrote: > > > > > On Sun, 2020-07-26 at 01:35 +0200, SeongJae Park wrote: > > > > On Sat, 25 Jul 2020 10:29:23 -0700 Joe Perches <joe@perches.com> wrote: > > > > > > > > > On Sat, 2020-07-25 at 15:02 +0200, Michał Mirosław wrote: > > > > > > Hello, > > > > > > > > > > > > I see that this patch went into next and is already inciting people to > > > > > > do wrong things [1]. Can you please fix it to require '--subjective' > > > > > > switch or otherwise mark it clearly as suggestion-only? > > > > > > > > > > > > The coding-style as in Linus' master says about *NEW* uses of the words > > > > > > listed (those introductions I expect to be actually rare) and not about > > > > > > existing use in the code or industry. Making a noise about all uses > > > > > > found surely will generate a lot more irrelevant patches. > > > > > > > > > > > > [1] https://www.spinics.net/lists/linux-tegra/msg51849.html > > > > > > > > > > And if not reverted, perhaps do not check existing files > > > > > at all but only check patches and change the message to > > > > > show only suggestions not from a specification. > > > > > > > > Agreed for this case. However, excluding existing file check doesn't fully > > > > avoid this problem. Also, more terms having different deprecation rules might > > > > be added in future. How about allowing file check but show reference in the > > > > suggestion message as below? > > > > > > The general problem is that drivers/staging, net/ and drivers/net > > > all have --strict on by default. > > > > > > Emitting these deprecated terms messages with -f --file uses for > > > files in those directories isn't a great idea. > > > > Thank you for kindly explaining your concenrs in detail. However, I think it's > > ok to do this check even without '--strict' for files if we explicitly says > > it's suggestion only, as Michal said. My patch does so. > > > > > > diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt > > > [] > > > > @@ -3,8 +3,10 @@ > > > > # The format of each line is: > > > > # deprecated||suggested > > > > # > > > > +# If special rules are applied on the terms, please comment those. > > > > > > Disagree. Comments about these existing uses aren't helpful. > > > > Sorry, I don't understand your point here. Why do you think it's not helpful? > > If 'checkpatch' finds the deprecated terms, it will ask people to read this > > file, which explains special rules for each of the deprecations if exists. The > > rule is, in the case of 'slave', 'applies to new uses only'. Therefore, people > > could stop sending the noisy unnecessary patches to the maintainers. > > Because it will describe this for _every_ instance > of any deprecated word in the file. Thank you for kindly explaining your concern. I personally thought the verbose warning is not a real problem. Anyway, how about below patch, then? It will show only one warning or check for each of the terms. ================================= >8 ========================================== From 6c606c62ea25933db8bb0afec083b5b4b8b3f11f Mon Sep 17 00:00:00 2001 From: SeongJae Park <sjpark@amazon.de> Date: Sun, 26 Jul 2020 01:14:48 +0200 Subject: [PATCH] scripts/deprecatd_terms: provide references Deprecation of terms could have special rules. For example, 'slave' is ok for existing usages. Same to 'master', but it's also ok unless it's used with 'slave'. This commit provides the references for such rules. Also, because the report became more verbose a little, this commit makes the report to be made for only one instance of each deprecated term. Signed-off-by: SeongJae Park <sjpark@amazon.de> --- scripts/checkpatch.pl | 6 +++++- scripts/deprecated_terms.txt | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e9fde28eb0de..227e088bfe56 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -721,6 +721,7 @@ sub read_word_corrections { my %deprecated_terms_fix; read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix); my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix; +my %deprecated_terms_reported = map { $_ => 1 } # Load common spelling mistakes and build regular expression list. my $misspellings; @@ -2975,13 +2976,16 @@ sub process { ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { my $deprecated_term = $1; + last if (exists($deprecated_terms_reported{$deprecated_term})); + $deprecated_terms_reported{$deprecated_term} = 1; + my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("DEPRECATED_TERM", - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && + "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead. See: scripts/deprecated_terms.txt\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; } diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt index 1be27a24187b..d92b9c896fce 100644 --- a/scripts/deprecated_terms.txt +++ b/scripts/deprecated_terms.txt @@ -3,8 +3,10 @@ # The format of each line is: # deprecated||suggested # +# If special rules are applied on the terms, please comment those. +# +# Refer to "4) Naming" section of Documentation/process/coding-style.rst for +# below three terms. blacklist||(denylist|blocklist) -# For other alternatives of 'slave', Please refer to -# Documentation/process/coding-style.rst slave||(secondary|target|...) whitelist||(allowlist|passlist) -- 2.17.1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 7:45 ` SeongJae Park @ 2020-07-26 14:50 ` Joe Perches 2020-07-26 15:36 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-07-26 14:50 UTC (permalink / raw) To: SeongJae Park Cc: Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 2020-07-26 at 09:45 +0200, SeongJae Park wrote: > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -721,6 +721,7 @@ sub read_word_corrections { > my %deprecated_terms_fix; > read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix); > my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix; > +my %deprecated_terms_reported = map { $_ => 1 } overly verbose naming and this doesn't need initialization here. > @@ -2975,13 +2976,16 @@ sub process { > ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { > while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { > my $deprecated_term = $1; > + last if (exists($deprecated_terms_reported{$deprecated_term})); next if (...) to check if multiple terms exists on the same line > + $deprecated_terms_reported{$deprecated_term} = 1; > + But this does need to be reset to empty when checking the next file > my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; > $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); > $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); > my $msg_level = \&WARN; > $msg_level = \&CHK if ($file); > if (&{$msg_level}("DEPRECATED_TERM", > - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && > + "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead. See: scripts/deprecated_terms.txt\n" . $herecurr) && > $fix) { > $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; I think it simpler to avoid emitting this on existing files. I do not want to encourage relatively inexperienced people to run checkpatch and submit inappropriate patches. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 14:50 ` Joe Perches @ 2020-07-26 15:36 ` SeongJae Park 2020-07-26 16:42 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-07-26 15:36 UTC (permalink / raw) To: Joe Perches Cc: SeongJae Park, Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@perches.com> wrote: > On Sun, 2020-07-26 at 09:45 +0200, SeongJae Park wrote: > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -721,6 +721,7 @@ sub read_word_corrections { > > my %deprecated_terms_fix; > > read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix); > > my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix; > > +my %deprecated_terms_reported = map { $_ => 1 } > > overly verbose naming and this doesn't need initialization here. > > > @@ -2975,13 +2976,16 @@ sub process { > > ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { > > while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { > > my $deprecated_term = $1; > > + last if (exists($deprecated_terms_reported{$deprecated_term})); > > next if (...) to check if multiple terms exists on the same line Agreed on these comments, thanks! > > > + $deprecated_terms_reported{$deprecated_term} = 1; > > + > > But this does need to be reset to empty when checking the next file Hmm... I though you mean reporting same term multiple times too verbose... Did I misunderstand your point? > > > my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; > > $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); > > $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); > > my $msg_level = \&WARN; > > $msg_level = \&CHK if ($file); > > if (&{$msg_level}("DEPRECATED_TERM", > > - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && > > + "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead. See: scripts/deprecated_terms.txt\n" . $herecurr) && > > $fix) { > > $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; > > I think it simpler to avoid emitting this on existing files. Agreed, it's much simpler. However, my concerns on excluding existing file checks are: 1. Avoiding existing file checks will still not stop warning patches mentioning existing deprecated terms. 2. If the term mistakenly comes in newly, it would be hard to check it later. 3. Some future deprecations of terms might be applied to existing uses, as 's/fuck/hug' did. > > I do not want to encourage relatively inexperienced people > to run checkpatch and submit inappropriate patches. Me, neither. But, I think providing more warnings and references is better for that. Experienced people would be able to easily ignore the false positives. Simply limiting checks could allow people submitting inappropriate patches intorducing new uses of deprecated terms. Thanks, SeongJae Park ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 15:36 ` SeongJae Park @ 2020-07-26 16:42 ` Joe Perches 2020-07-26 18:07 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-07-26 16:42 UTC (permalink / raw) To: SeongJae Park Cc: Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 2020-07-26 at 17:36 +0200, SeongJae Park wrote: > On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@perches.com> wrote: [] > > I do not want to encourage relatively inexperienced people > > to run checkpatch and submit inappropriate patches. > > Me, neither. But, I think providing more warnings and references is better for > that. Unfortunately, the inexperienced _do_ in fact run checkpatch on files and submit inappropriate patches. It's generally a time sink for the experienced maintainers to reply. > Simply limiting checks could allow people submitting inappropriate patches > intorducing new uses of deprecated terms. Tradeoffs... I expect that patches being reviewed by maintainers are preferred over files being inappropriately changed by the inexperienced. Those inappropriate changes should not be encouraged by tools placed in the hands of the inexperienced. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking 2020-07-26 16:42 ` Joe Perches @ 2020-07-26 18:07 ` SeongJae Park 2020-07-26 20:33 ` Michał Mirosław 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-07-26 18:07 UTC (permalink / raw) To: Joe Perches Cc: SeongJae Park, Michał Mirosław, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 26 Jul 2020 09:42:06 -0700 Joe Perches <joe@perches.com> wrote: > On Sun, 2020-07-26 at 17:36 +0200, SeongJae Park wrote: > > On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@perches.com> wrote: > [] > > > I do not want to encourage relatively inexperienced people > > > to run checkpatch and submit inappropriate patches. > > > > Me, neither. But, I think providing more warnings and references is better for > > that. > > Unfortunately, the inexperienced _do_ in fact run > checkpatch on files and submit inappropriate patches. > > It's generally a time sink for the experienced > maintainers to reply. > > > Simply limiting checks could allow people submitting inappropriate patches > > intorducing new uses of deprecated terms. > > Tradeoffs... > > I expect that patches being reviewed by maintainers > are preferred over files being inappropriately changed > by the inexperienced. > > Those inappropriate changes should not be encouraged > by tools placed in the hands of the inexperienced. Right, many things are tradeoff. Seems we arrived in the point, though we still have different opinions. To summarize the pros and cons of my patch from my perspective: Pros 1: Handle future terms deprecated with different reasons and coverages. Pros 2: Inappropriate patches are avoided if the submitters carefully read the warning messages. Cons: Careless people could still bother maintainers by not carefully reading the message and sending inappropriate patches. To me, the pros still seems larger than the cons. I would like to also again mention that the maintainer who first reported the problem, Michal, told it's ok with the explicit messaging. Nonethelss, this is just my opinion. Attaching the patch addressing your comments for the previous version. The changes from the previous version are: - Make the name of reported terms not too verbose - Avoid unnecessary initialization of the reported terms hash - Warn multiple deprecated terms in same line Thanks, SeongJae Park p.s I modified my mail formatter to skip adding 'Re:' in the subject. I thought it's usual behavior of the mailers, but seems it made you only annoying, sorry. I will not add more 'Re:' for you. =================================== >8 ======================================== From 169939e24ae98125efcf3af024e6e09cf5cd85f0 Mon Sep 17 00:00:00 2001 From: SeongJae Park <sjpark@amazon.de> Date: Sun, 26 Jul 2020 01:14:48 +0200 Subject: [PATCH v3] scripts/deprecatd_terms: provide references Deprecation of terms could have special rules. For example, 'slave' is ok for existing usages. Same to 'master', but it's also ok unless it's used with 'slave'. This commit provides the references for such rules. Also, because the report became more verbose a little, this commit makes the report to be made for only one instance of each deprecated term. Signed-off-by: SeongJae Park <sjpark@amazon.de> --- scripts/checkpatch.pl | 6 +++++- scripts/deprecated_terms.txt | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e9fde28eb0de..abed47647fb0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -721,6 +721,7 @@ sub read_word_corrections { my %deprecated_terms_fix; read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix); my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix; +my %deprecates_reported = map # Load common spelling mistakes and build regular expression list. my $misspellings; @@ -2975,13 +2976,16 @@ sub process { ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { my $deprecated_term = $1; + next if (exists($deprecates_reported{$deprecated_term})); + $deprecates_reported{$deprecated_term} = 1; + my $suggested = $deprecated_terms_fix{lc($deprecated_term)}; $suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/); $suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/); my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("DEPRECATED_TERM", - "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) && + "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead. See: scripts/deprecated_terms.txt\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/; } diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt index 1be27a24187b..d92b9c896fce 100644 --- a/scripts/deprecated_terms.txt +++ b/scripts/deprecated_terms.txt @@ -3,8 +3,10 @@ # The format of each line is: # deprecated||suggested # +# If special rules are applied on the terms, please comment those. +# +# Refer to "4) Naming" section of Documentation/process/coding-style.rst for +# below three terms. blacklist||(denylist|blocklist) -# For other alternatives of 'slave', Please refer to -# Documentation/process/coding-style.rst slave||(secondary|target|...) whitelist||(allowlist|passlist) -- 2.17.1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-26 18:07 ` SeongJae Park @ 2020-07-26 20:33 ` Michał Mirosław 2020-07-27 6:54 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Michał Mirosław @ 2020-07-26 20:33 UTC (permalink / raw) To: SeongJae Park Cc: Joe Perches, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, Jul 26, 2020 at 08:07:48PM +0200, SeongJae Park wrote: > On Sun, 26 Jul 2020 09:42:06 -0700 Joe Perches <joe@perches.com> wrote: > > > On Sun, 2020-07-26 at 17:36 +0200, SeongJae Park wrote: > > > On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@perches.com> wrote: > > [] > > > > I do not want to encourage relatively inexperienced people > > > > to run checkpatch and submit inappropriate patches. > > > > > > Me, neither. But, I think providing more warnings and references is better for > > > that. > > > > Unfortunately, the inexperienced _do_ in fact run > > checkpatch on files and submit inappropriate patches. > > > > It's generally a time sink for the experienced > > maintainers to reply. > > > > > Simply limiting checks could allow people submitting inappropriate patches > > > intorducing new uses of deprecated terms. > > > > Tradeoffs... > > > > I expect that patches being reviewed by maintainers > > are preferred over files being inappropriately changed > > by the inexperienced. > > > > Those inappropriate changes should not be encouraged > > by tools placed in the hands of the inexperienced. > > Right, many things are tradeoff. Seems we arrived in the point, though we > still have different opinions. To summarize the pros and cons of my patch from > my perspective: > > Pros 1: Handle future terms deprecated with different reasons and coverages. > Pros 2: Inappropriate patches are avoided if the submitters carefully read the > warning messages. > Cons: Careless people could still bother maintainers by not carefully reading > the message and sending inappropriate patches. > > To me, the pros still seems larger than the cons. I would like to also again > mention that the maintainer who first reported the problem, Michal, told it's > ok with the explicit messaging. Nonethelss, this is just my opinion. > > Attaching the patch addressing your comments for the previous version. The > changes from the previous version are: > > - Make the name of reported terms not too verbose > - Avoid unnecessary initialization of the reported terms hash > - Warn multiple deprecated terms in same line Hi, Maybe you could split the meaning of --subjective and --strict, and enable those checks only for --subjective? The test is really hard to do right: you would have to consider the context and not only mere occurrence of a word (heh, I even wrote 'blacklisted' here, since it really is about a night/danger analogy and not political/ethical one). Best Regards, Michał Mirosław ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-26 20:33 ` Michał Mirosław @ 2020-07-27 6:54 ` SeongJae Park 2020-07-27 20:44 ` Andrew Morton 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-07-27 6:54 UTC (permalink / raw) To: Michał Mirosław Cc: SeongJae Park, Joe Perches, SeongJae Park, Andrew Morton, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Sun, 26 Jul 2020 22:33:28 +0200 "Michał Mirosław" <mirq-linux@rere.qmqm.pl> wrote: > On Sun, Jul 26, 2020 at 08:07:48PM +0200, SeongJae Park wrote: > > On Sun, 26 Jul 2020 09:42:06 -0700 Joe Perches <joe@perches.com> wrote: > > > > > On Sun, 2020-07-26 at 17:36 +0200, SeongJae Park wrote: > > > > On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@perches.com> wrote: > > > [] > > > > > I do not want to encourage relatively inexperienced people > > > > > to run checkpatch and submit inappropriate patches. > > > > > > > > Me, neither. But, I think providing more warnings and references is better for > > > > that. > > > > > > Unfortunately, the inexperienced _do_ in fact run > > > checkpatch on files and submit inappropriate patches. > > > > > > It's generally a time sink for the experienced > > > maintainers to reply. > > > > > > > Simply limiting checks could allow people submitting inappropriate patches > > > > intorducing new uses of deprecated terms. > > > > > > Tradeoffs... > > > > > > I expect that patches being reviewed by maintainers > > > are preferred over files being inappropriately changed > > > by the inexperienced. > > > > > > Those inappropriate changes should not be encouraged > > > by tools placed in the hands of the inexperienced. > > > > Right, many things are tradeoff. Seems we arrived in the point, though we > > still have different opinions. To summarize the pros and cons of my patch from > > my perspective: > > > > Pros 1: Handle future terms deprecated with different reasons and coverages. > > Pros 2: Inappropriate patches are avoided if the submitters carefully read the > > warning messages. > > Cons: Careless people could still bother maintainers by not carefully reading > > the message and sending inappropriate patches. > > > > To me, the pros still seems larger than the cons. I would like to also again > > mention that the maintainer who first reported the problem, Michal, told it's > > ok with the explicit messaging. Nonethelss, this is just my opinion. > > > > Attaching the patch addressing your comments for the previous version. The > > changes from the previous version are: > > > > - Make the name of reported terms not too verbose > > - Avoid unnecessary initialization of the reported terms hash > > - Warn multiple deprecated terms in same line > > Hi, > > Maybe you could split the meaning of --subjective and --strict, and > enable those checks only for --subjective? The test is really hard to do > right: you would have to consider the context and not only mere occurrence > of a word (heh, I even wrote 'blacklisted' here, since it really is about > a night/danger analogy and not political/ethical one). I'm concerning if applying the switch and making this patch non-default could reduce the check coverage. Moreover, IMHO, the deprecation rule of the terms that described in the 'coding-style.rst' is not so subjective but clear. Also, the checkpatch's warning/check message for those seems explicit enough to me. And, the deprecated terms feature is not for this specific terms (master/slave/blacklist/whitelist) only but general deprecated terms. Maybe we could add one more rule in the 'deprecated_terms.txt' by adding a comment, say, "Please add only terms that deprecated with clear rules", for avoiding introduce of subjective deprecated terms in future, though. Thanks, SeongJae Park > > Best Regards, > Michał Mirosław ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-27 6:54 ` SeongJae Park @ 2020-07-27 20:44 ` Andrew Morton 2020-07-27 20:49 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: Andrew Morton @ 2020-07-27 20:44 UTC (permalink / raw) To: SeongJae Park Cc: Michał Mirosław, SeongJae Park, Joe Perches, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Mon, 27 Jul 2020 08:54:41 +0200 SeongJae Park <sjpark@amazon.com> wrote: > > > > Unfortunately, the inexperienced _do_ in fact run > > > > checkpatch on files and submit inappropriate patches. I don't think I really agree with the "new code only" guideline (where did this come from, anyway?). 10 years from now any remaining pre-2020 terms will look exceedingly archaic and will get converted at some point. Wouldn't be longterm realistic to just bite the bullet now and add these conversions to the various todo lists? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-27 20:44 ` Andrew Morton @ 2020-07-27 20:49 ` Joe Perches 2020-07-28 6:22 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-07-27 20:49 UTC (permalink / raw) To: Andrew Morton, SeongJae Park Cc: Michał Mirosław, SeongJae Park, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park On Mon, 2020-07-27 at 13:44 -0700, Andrew Morton wrote: > On Mon, 27 Jul 2020 08:54:41 +0200 SeongJae Park <sjpark@amazon.com> wrote: > > > > > > Unfortunately, the inexperienced _do_ in fact run > > > > > checkpatch on files and submit inappropriate patches. > > I don't think I really agree with the "new code only" guideline (where > did this come from, anyway?). 10 years from now any remaining pre-2020 > terms will look exceedingly archaic and will get converted at some > point. > > Wouldn't be longterm realistic to just bite the bullet now and add these > conversions to the various todo lists? I don't think so. There's no exclusion list for existing uses written to external specification. It's just emitting effectively noisy warnings on things that should not be changed. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: checkpatch: support deprecated terms checking 2020-07-27 20:49 ` Joe Perches @ 2020-07-28 6:22 ` SeongJae Park 0 siblings, 0 replies; 35+ messages in thread From: SeongJae Park @ 2020-07-28 6:22 UTC (permalink / raw) To: Joe Perches Cc: Andrew Morton, SeongJae Park, Michał Mirosław, SeongJae Park, linux-kernel, apw, colin.king, jslaby, pavel, SeongJae Park, dan.j.williams, corbet, gregkh, mishi, skhan On Mon, 27 Jul 2020 13:49:00 -0700 Joe Perches <joe@perches.com> wrote: > On Mon, 2020-07-27 at 13:44 -0700, Andrew Morton wrote: > > On Mon, 27 Jul 2020 08:54:41 +0200 SeongJae Park <sjpark@amazon.com> wrote: > > > > > > > > Unfortunately, the inexperienced _do_ in fact run > > > > > > checkpatch on files and submit inappropriate patches. > > > > I don't think I really agree with the "new code only" guideline (where > > did this come from, anyway?). 10 years from now any remaining pre-2020 > > terms will look exceedingly archaic and will get converted at some > > point. > > > > Wouldn't be longterm realistic to just bite the bullet now and add these > > conversions to the various todo lists? > > I don't think so. > > There's no exclusion list for existing uses > written to external specification. > > It's just emitting effectively noisy warnings > on things that should not be changed. > Just noticed that this patchset and the followup[1] for sync with inclusive terms commit[2] are dropped from -mm tree. I admit it could generate some false positive warnings, though my followup patch[3] makes the message noisy but gives clear references. I still believe it's better to provide the messages, but I also know people could think differently. After all, the biggest part of the initial goal of this patches is already made by the inclusive terms commit[2]. So, I would respect the decision. [1] https://lore.kernel.org/lkml/20200713071912.24432-1-sjpark@amazon.com/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/process/coding-style.rst?id=a5f526ecb075a08c4a082355020166c7fe13ae27 [3] https://lore.kernel.org/lkml/20200726180748.29924-1-sj38.park@gmail.com/ Thanks, SeongJae Park ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v4 2/2] scripts/deprecated_terms: Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 6:25 [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park 2020-06-11 6:25 ` [PATCH v4 1/2] checkpatch: support deprecated terms checking SeongJae Park @ 2020-06-11 6:25 ` SeongJae Park 2020-06-11 6:35 ` [PATCH v4 0/2] " Joe Perches 2 siblings, 0 replies; 35+ messages in thread From: SeongJae Park @ 2020-06-11 6:25 UTC (permalink / raw) To: akpm, apw, joe; +Cc: SeongJae Park, colin.king, sj38.park, jslaby, linux-kernel From: SeongJae Park <sjpark@amazon.de> This commit recommends the patches to replace 'blacklist' and 'whitelist' with the 'denylist' and 'allowlist', because the new suggestions are incontrovertible, doesn't make people hurt, and more self-explanatory. Signed-off-by: SeongJae Park <sjpark@amazon.de> --- scripts/deprecated_terms.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/deprecated_terms.txt b/scripts/deprecated_terms.txt index 6faa06451c3d..4512ef5d5ffa 100644 --- a/scripts/deprecated_terms.txt +++ b/scripts/deprecated_terms.txt @@ -3,3 +3,5 @@ # The format of each line is: # deprecated||suggested # +blacklist||denylist +whitelist||allowlist -- 2.17.1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 6:25 [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park 2020-06-11 6:25 ` [PATCH v4 1/2] checkpatch: support deprecated terms checking SeongJae Park 2020-06-11 6:25 ` [PATCH v4 2/2] scripts/deprecated_terms: Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park @ 2020-06-11 6:35 ` Joe Perches 2020-06-11 7:38 ` SeongJae Park 2 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-06-11 6:35 UTC (permalink / raw) To: SeongJae Park, akpm, apw Cc: SeongJae Park, colin.king, sj38.park, jslaby, linux-kernel On Thu, 2020-06-11 at 08:25 +0200, SeongJae Park wrote: > From: SeongJae Park <sjpark@amazon.de> > > This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' > and 2) set the 'blacklist' and 'whitelist' as deprecated with > replacement suggestion of 'denylist' and 'allowlist', because the > suggestions are incontrovertible, doesn't make people hurt, and more > self-explanatory. While the checkpatch implementation is better, I'm still very "meh" about the whole concept. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 6:35 ` [PATCH v4 0/2] " Joe Perches @ 2020-06-11 7:38 ` SeongJae Park 2020-06-11 8:16 ` Jiri Slaby 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-06-11 7:38 UTC (permalink / raw) To: Joe Perches Cc: SeongJae Park, akpm, apw, SeongJae Park, colin.king, sj38.park, jslaby, linux-kernel On Wed, 10 Jun 2020 23:35:24 -0700 Joe Perches <joe@perches.com> wrote: > On Thu, 2020-06-11 at 08:25 +0200, SeongJae Park wrote: > > From: SeongJae Park <sjpark@amazon.de> > > > > This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' > > and 2) set the 'blacklist' and 'whitelist' as deprecated with > > replacement suggestion of 'denylist' and 'allowlist', because the > > suggestions are incontrovertible, doesn't make people hurt, and more > > self-explanatory. > > While the checkpatch implementation is better, > I'm still very "meh" about the whole concept. I can understand your concerns about politic things in the second patch. However, the concept of the 'deprecated terms' in the first patch is not political but applicable to the general cases. We already had the commits[1] for a similar case. So, could you ack for at least the first patch? [1] https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Hugs Thanks, SeongJae Park ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 7:38 ` SeongJae Park @ 2020-06-11 8:16 ` Jiri Slaby 2020-06-11 8:30 ` SeongJae Park 2020-06-12 14:40 ` Michael Ellerman 0 siblings, 2 replies; 35+ messages in thread From: Jiri Slaby @ 2020-06-11 8:16 UTC (permalink / raw) To: SeongJae Park, Joe Perches Cc: akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel On 11. 06. 20, 9:38, SeongJae Park wrote: > On Wed, 10 Jun 2020 23:35:24 -0700 Joe Perches <joe@perches.com> wrote: > >> On Thu, 2020-06-11 at 08:25 +0200, SeongJae Park wrote: >>> From: SeongJae Park <sjpark@amazon.de> >>> >>> This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' >>> and 2) set the 'blacklist' and 'whitelist' as deprecated with >>> replacement suggestion of 'denylist' and 'allowlist', because the >>> suggestions are incontrovertible, doesn't make people hurt, and more >>> self-explanatory. >> >> While the checkpatch implementation is better, >> I'm still very "meh" about the whole concept. > > I can understand your concerns about politic things in the second patch. > However, the concept of the 'deprecated terms' in the first patch is not > political but applicable to the general cases. We already had the commits[1] > for a similar case. So, could you ack for at least the first patch? > > [1] https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Hugs Fuck you! replaced by hug you! is a completely different story. The former is indeed offending to majority (despite it's quite common to tell someone "fuck you" in my subregion; OTOH hugging, no way -- I'm a straight non-communist). If it turns out that any word (e.g. blacklist) offends _majority_ (or at least a significant part of it) of some minority or culture, then sure, we should send it to /dev/null. But we should by no means listen to extreme individuals. thanks, -- js suse labs ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 8:16 ` Jiri Slaby @ 2020-06-11 8:30 ` SeongJae Park 2020-06-11 8:32 ` Jiri Slaby 2020-06-12 14:40 ` Michael Ellerman 1 sibling, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-06-11 8:30 UTC (permalink / raw) To: Jiri Slaby Cc: SeongJae Park, Joe Perches, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel, kristen.c.accardi, mishi, skhan, gregkh On Thu, 11 Jun 2020 10:16:09 +0200 Jiri Slaby <jslaby@suse.cz> wrote: > On 11. 06. 20, 9:38, SeongJae Park wrote: > > On Wed, 10 Jun 2020 23:35:24 -0700 Joe Perches <joe@perches.com> wrote: > > > >> On Thu, 2020-06-11 at 08:25 +0200, SeongJae Park wrote: > >>> From: SeongJae Park <sjpark@amazon.de> > >>> > >>> This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' > >>> and 2) set the 'blacklist' and 'whitelist' as deprecated with > >>> replacement suggestion of 'denylist' and 'allowlist', because the > >>> suggestions are incontrovertible, doesn't make people hurt, and more > >>> self-explanatory. > >> > >> While the checkpatch implementation is better, > >> I'm still very "meh" about the whole concept. > > > > I can understand your concerns about politic things in the second patch. > > However, the concept of the 'deprecated terms' in the first patch is not > > political but applicable to the general cases. We already had the commits[1] > > for a similar case. So, could you ack for at least the first patch? > > > > [1] https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Hugs > > Fuck you! replaced by hug you! is a completely different story. The > former is indeed offending to majority (despite it's quite common to > tell someone "fuck you" in my subregion; OTOH hugging, no way -- I'm a > straight non-communist). If it turns out that any word (e.g. blacklist) > offends _majority_ (or at least a significant part of it) of some > minority or culture, then sure, we should send it to /dev/null. But we > should by no means listen to extreme individuals. Thank you for the opinion. But, my point here is, deprecating some terms would occur in general as the f-word to hug replacement was, and the first patch is a simple technical preparation for such case. And, therefore, it would not need to be blocked due to the second patch. For example, as it seems at least you and I agree on the f-word to hug replacement, we could add ``fuck||hug`` in the `deprecated_terms.txt` file to avoid future spread of the f-words. Also, I personally don't think the second patch as a political extreme change but just a right thing to do. Nonetheless, I also understand people could think in different ways. Moreover, it is obviously non-technical thing which I am really bad at. For the reason, I am CC-ing the code of conduct committees[1]. I would like to hear their opinions on this. [1] https://www.kernel.org/code-of-conduct.html Thanks, SeongJae Park > > thanks, > -- > js > suse labs ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 8:30 ` SeongJae Park @ 2020-06-11 8:32 ` Jiri Slaby 2020-06-11 10:43 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: Jiri Slaby @ 2020-06-11 8:32 UTC (permalink / raw) To: SeongJae Park Cc: Joe Perches, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel, kristen.c.accardi, mishi, skhan, gregkh On 11. 06. 20, 10:30, SeongJae Park wrote: > For example, as it seems at least you and I agree on the f-word to hug > replacement, we could add ``fuck||hug`` in the `deprecated_terms.txt` file to > avoid future spread of the f-words. You will likely get my ACK on that, if that helps. thanks, -- js suse labs ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 8:32 ` Jiri Slaby @ 2020-06-11 10:43 ` Joe Perches 2020-06-12 6:40 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-06-11 10:43 UTC (permalink / raw) To: Jiri Slaby, SeongJae Park Cc: akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel, kristen.c.accardi, mishi, skhan, gregkh On Thu, 2020-06-11 at 10:32 +0200, Jiri Slaby wrote: > On 11. 06. 20, 10:30, SeongJae Park wrote: > > For example, as it seems at least you and I agree on the f-word to hug > > replacement, we could add ``fuck||hug`` in the `deprecated_terms.txt` file to > > avoid future spread of the f-words. > > You will likely get my ACK on that, if that helps. But likely not mine. $ git grep -i '\bfuck' | wc -l 15 ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 10:43 ` Joe Perches @ 2020-06-12 6:40 ` SeongJae Park 2020-06-12 7:05 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-06-12 6:40 UTC (permalink / raw) To: Joe Perches Cc: Jiri Slaby, SeongJae Park, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel, kristen.c.accardi, mishi, skhan, gregkh On Thu, 11 Jun 2020 03:43:32 -0700 Joe Perches <joe@perches.com> wrote: > On Thu, 2020-06-11 at 10:32 +0200, Jiri Slaby wrote: > > On 11. 06. 20, 10:30, SeongJae Park wrote: > > > For example, as it seems at least you and I agree on the f-word to hug > > > replacement, we could add ``fuck||hug`` in the `deprecated_terms.txt` file to > > > avoid future spread of the f-words. > > > > You will likely get my ACK on that, if that helps. > > But likely not mine. It's ok, I will respect every opinion. But... > > $ git grep -i '\bfuck' | wc -l > 15 I'm not understanding what you're meaning with this. Could I ask your explanation, please? Thanks, SeongJae Park ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-12 6:40 ` SeongJae Park @ 2020-06-12 7:05 ` Joe Perches 0 siblings, 0 replies; 35+ messages in thread From: Joe Perches @ 2020-06-12 7:05 UTC (permalink / raw) To: SeongJae Park Cc: Jiri Slaby, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel, kristen.c.accardi, mishi, skhan, gregkh On Fri, 2020-06-12 at 08:40 +0200, SeongJae Park wrote: > On Thu, 11 Jun 2020 03:43:32 -0700 Joe Perches <joe@perches.com> wrote: > > On Thu, 2020-06-11 at 10:32 +0200, Jiri Slaby wrote: > > > On 11. 06. 20, 10:30, SeongJae Park wrote: > > > > For example, as it seems at least you and I agree on the f-word to hug > > > > replacement, we could add ``fuck||hug`` in the `deprecated_terms.txt` file to > > > > avoid future spread of the f-words. [] > > $ git grep -i '\bfuck' | wc -l > > 15 > > I'm not understanding what you're meaning with this. Could I ask your > explanation, please? Just that there are 15 instances of that word in the kernel. cheers, Joe ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-11 8:16 ` Jiri Slaby 2020-06-11 8:30 ` SeongJae Park @ 2020-06-12 14:40 ` Michael Ellerman 2020-06-14 21:29 ` Pavel Machek 1 sibling, 1 reply; 35+ messages in thread From: Michael Ellerman @ 2020-06-12 14:40 UTC (permalink / raw) To: Jiri Slaby, SeongJae Park, Joe Perches Cc: akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel Jiri Slaby <jslaby@suse.cz> writes: > On 11. 06. 20, 9:38, SeongJae Park wrote: >> On Wed, 10 Jun 2020 23:35:24 -0700 Joe Perches <joe@perches.com> wrote: >>> On Thu, 2020-06-11 at 08:25 +0200, SeongJae Park wrote: >>>> From: SeongJae Park <sjpark@amazon.de> >>>> >>>> This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' >>>> and 2) set the 'blacklist' and 'whitelist' as deprecated with >>>> replacement suggestion of 'denylist' and 'allowlist', because the >>>> suggestions are incontrovertible, doesn't make people hurt, and more >>>> self-explanatory. >>> >>> While the checkpatch implementation is better, >>> I'm still very "meh" about the whole concept. >> >> I can understand your concerns about politic things in the second patch. >> However, the concept of the 'deprecated terms' in the first patch is not >> political but applicable to the general cases. We already had the commits[1] >> for a similar case. So, could you ack for at least the first patch? >> >> [1] https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Hugs > > Fuck you! replaced by hug you! is a completely different story. The > former is indeed offending to majority (despite it's quite common to > tell someone "fuck you" in my subregion; OTOH hugging, no way -- I'm a > straight non-communist). If it turns out that any word (e.g. blacklist) > offends _majority_ (or at least a significant part of it) of some > minority or culture, then sure, we should send it to /dev/null. > should by no means listen to extreme individuals. I agree you have to draw the line somewhere, there will always be someone somewhere that's offended by something. But this seems like a pretty easy case. It's not like blacklist / whitelist are even good to begin with, it's not obvious which is which, you have to learn that black is bad and white is good. Blocklist (or denylist?) and allowlist are actually more descriptive and less likely to cause confusion. cheers ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-12 14:40 ` Michael Ellerman @ 2020-06-14 21:29 ` Pavel Machek 2020-06-15 4:21 ` Jiri Slaby 0 siblings, 1 reply; 35+ messages in thread From: Pavel Machek @ 2020-06-14 21:29 UTC (permalink / raw) To: Michael Ellerman Cc: Jiri Slaby, SeongJae Park, Joe Perches, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2392 bytes --] On Sat 2020-06-13 00:40:59, Michael Ellerman wrote: > Jiri Slaby <jslaby@suse.cz> writes: > > On 11. 06. 20, 9:38, SeongJae Park wrote: > >> On Wed, 10 Jun 2020 23:35:24 -0700 Joe Perches <joe@perches.com> wrote: > >>> On Thu, 2020-06-11 at 08:25 +0200, SeongJae Park wrote: > >>>> From: SeongJae Park <sjpark@amazon.de> > >>>> > >>>> This patchset 1) adds support of deprecated terms in the 'checkpatch.pl' > >>>> and 2) set the 'blacklist' and 'whitelist' as deprecated with > >>>> replacement suggestion of 'denylist' and 'allowlist', because the > >>>> suggestions are incontrovertible, doesn't make people hurt, and more > >>>> self-explanatory. > >>> > >>> While the checkpatch implementation is better, > >>> I'm still very "meh" about the whole concept. > >> > >> I can understand your concerns about politic things in the second patch. > >> However, the concept of the 'deprecated terms' in the first patch is not > >> political but applicable to the general cases. We already had the commits[1] > >> for a similar case. So, could you ack for at least the first patch? > >> > >> [1] https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Hugs > > > > Fuck you! replaced by hug you! is a completely different story. The > > former is indeed offending to majority (despite it's quite common to > > tell someone "fuck you" in my subregion; OTOH hugging, no way -- I'm a > > straight non-communist). If it turns out that any word (e.g. blacklist) > > offends _majority_ (or at least a significant part of it) of some > > minority or culture, then sure, we should send it to /dev/null. > > should by no means listen to extreme individuals. > > I agree you have to draw the line somewhere, there will always be > someone somewhere that's offended by something. But this seems like a > pretty easy case. > > It's not like blacklist / whitelist are even good to begin with, it's > not obvious which is which, you have to learn that black is bad and > white is good. > > Blocklist (or denylist?) and allowlist are actually more descriptive and > less likely to cause confusion. You do not understand how word "blacklist" is used inside the kernel, do you? Do a quick grep. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-14 21:29 ` Pavel Machek @ 2020-06-15 4:21 ` Jiri Slaby 2020-06-15 6:12 ` Pavel Machek 0 siblings, 1 reply; 35+ messages in thread From: Jiri Slaby @ 2020-06-15 4:21 UTC (permalink / raw) To: Pavel Machek, Michael Ellerman Cc: SeongJae Park, Joe Perches, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel On 14. 06. 20, 23:29, Pavel Machek wrote: >> I agree you have to draw the line somewhere, there will always be >> someone somewhere that's offended by something. But this seems like a >> pretty easy case. Yes, hence I left up to the minority and/or the touched culture. >> It's not like blacklist / whitelist are even good to begin with, it's >> not obvious which is which, you have to learn that black is bad and >> white is good. >> >> Blocklist (or denylist?) and allowlist are actually more descriptive and >> less likely to cause confusion. > > You do not understand how word "blacklist" is used inside the kernel, > do you? Do a quick grep. And now, do the same for "blocklist". And is "denylist" a proper word? As grep gives zarro results... It's not that easy to find alternatives. OTOH, admittedly, "blacklist" is used improperly in some contexts. Some synonyms fit better. thanks, -- js suse labs ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-15 4:21 ` Jiri Slaby @ 2020-06-15 6:12 ` Pavel Machek 2020-06-15 6:46 ` SeongJae Park 0 siblings, 1 reply; 35+ messages in thread From: Pavel Machek @ 2020-06-15 6:12 UTC (permalink / raw) To: Jiri Slaby Cc: Michael Ellerman, SeongJae Park, Joe Perches, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1115 bytes --] On Mon 2020-06-15 06:21:43, Jiri Slaby wrote: > On 14. 06. 20, 23:29, Pavel Machek wrote: > >> It's not like blacklist / whitelist are even good to begin with, it's > >> not obvious which is which, you have to learn that black is bad and > >> white is good. > >> > >> Blocklist (or denylist?) and allowlist are actually more descriptive and > >> less likely to cause confusion. > > > > You do not understand how word "blacklist" is used inside the kernel, > > do you? Do a quick grep. > > And now, do the same for "blocklist". > > And is "denylist" a proper word? As grep gives zarro results... > > It's not that easy to find alternatives. OTOH, admittedly, "blacklist" > is used improperly in some contexts. Some synonyms fit better. Well, many of the uses is "list of hardware that needs particular workaround" or "list of hardware that is broken in some way"... Neither 'blocklist' nor 'denylist' fit that usage. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-15 6:12 ` Pavel Machek @ 2020-06-15 6:46 ` SeongJae Park 2020-06-15 7:00 ` Joe Perches 0 siblings, 1 reply; 35+ messages in thread From: SeongJae Park @ 2020-06-15 6:46 UTC (permalink / raw) To: Pavel Machek Cc: Jiri Slaby, Michael Ellerman, SeongJae Park, Joe Perches, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel On Mon, 15 Jun 2020 08:12:08 +0200 Pavel Machek <pavel@ucw.cz> wrote: > > [-- Attachment #1: Type: text/plain, Size: 1115 bytes --] > > On Mon 2020-06-15 06:21:43, Jiri Slaby wrote: > > On 14. 06. 20, 23:29, Pavel Machek wrote: > > > >> It's not like blacklist / whitelist are even good to begin with, it's > > >> not obvious which is which, you have to learn that black is bad and > > >> white is good. > > >> > > >> Blocklist (or denylist?) and allowlist are actually more descriptive and > > >> less likely to cause confusion. > > > > > > You do not understand how word "blacklist" is used inside the kernel, > > > do you? Do a quick grep. I of course did grep of the terms before making this patchset. There are so many uses of the term, and therefore I thought it would be very hard and painful to replace the whole words. Of course, I also found some miuse of the terms and therefore I thought automatic scripting for the replacement also wouldn't make sense. That's why I made gives only warning to future patches. What this patch aims to do is avoiding the further spread of the terms, and incremental replacements to better terms, rather than the one point buggy and risky replacement. > > > > And now, do the same for "blocklist". > > > > And is "denylist" a proper word? As grep gives zarro results... > > > > It's not that easy to find alternatives. OTOH, admittedly, "blacklist" > > is used improperly in some contexts. Some synonyms fit better. > > Well, many of the uses is "list of hardware that needs particular > workaround" or "list of hardware that is broken in some > way"... Neither 'blocklist' nor 'denylist' fit that usage. Agreed, 'denylist' would also not fit in there. That said, this patchset will warn even such case so that people can think once again and find better term. So, I agree this patch is imperfect for many cases, but better than nothing. Thanks, SeongJae Park ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-15 6:46 ` SeongJae Park @ 2020-06-15 7:00 ` Joe Perches 2020-06-15 7:39 ` Pavel Machek 0 siblings, 1 reply; 35+ messages in thread From: Joe Perches @ 2020-06-15 7:00 UTC (permalink / raw) To: SeongJae Park, Pavel Machek Cc: Jiri Slaby, Michael Ellerman, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel On Mon, 2020-06-15 at 08:46 +0200, SeongJae Park wrote: > So, I agree this patch is imperfect for many cases, but better than nothing. Not necessarily. Having people strain for unusual equivalents to generally well known terms is not good. blacklist/whitelist is well understood. Would you do the same for red/black? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: Re: [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist 2020-06-15 7:00 ` Joe Perches @ 2020-06-15 7:39 ` Pavel Machek 0 siblings, 0 replies; 35+ messages in thread From: Pavel Machek @ 2020-06-15 7:39 UTC (permalink / raw) To: Joe Perches Cc: SeongJae Park, Jiri Slaby, Michael Ellerman, akpm, apw, SeongJae Park, colin.king, sj38.park, linux-kernel [-- Attachment #1: Type: text/plain, Size: 608 bytes --] On Mon 2020-06-15 00:00:56, Joe Perches wrote: > On Mon, 2020-06-15 at 08:46 +0200, SeongJae Park wrote: > > So, I agree this patch is imperfect for many cases, but better than nothing. > > Not necessarily. > > Having people strain for unusual equivalents > to generally well known terms is not good. Exactly. Plus consistency is good, and having same structure named blacklist here and naughtyhardwarelist there does not really help. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2020-07-28 6:23 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-11 6:25 [PATCH v4 0/2] Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park 2020-06-11 6:25 ` [PATCH v4 1/2] checkpatch: support deprecated terms checking SeongJae Park 2020-07-25 13:02 ` Michał Mirosław 2020-07-25 16:36 ` Joe Perches 2020-07-25 17:29 ` Joe Perches 2020-07-25 23:35 ` SeongJae Park 2020-07-26 4:27 ` Joe Perches 2020-07-26 7:18 ` SeongJae Park 2020-07-26 7:29 ` Joe Perches 2020-07-26 7:45 ` SeongJae Park 2020-07-26 14:50 ` Joe Perches 2020-07-26 15:36 ` SeongJae Park 2020-07-26 16:42 ` Joe Perches 2020-07-26 18:07 ` SeongJae Park 2020-07-26 20:33 ` Michał Mirosław 2020-07-27 6:54 ` SeongJae Park 2020-07-27 20:44 ` Andrew Morton 2020-07-27 20:49 ` Joe Perches 2020-07-28 6:22 ` SeongJae Park 2020-06-11 6:25 ` [PATCH v4 2/2] scripts/deprecated_terms: Recommend denylist/allowlist instead of blacklist/whitelist SeongJae Park 2020-06-11 6:35 ` [PATCH v4 0/2] " Joe Perches 2020-06-11 7:38 ` SeongJae Park 2020-06-11 8:16 ` Jiri Slaby 2020-06-11 8:30 ` SeongJae Park 2020-06-11 8:32 ` Jiri Slaby 2020-06-11 10:43 ` Joe Perches 2020-06-12 6:40 ` SeongJae Park 2020-06-12 7:05 ` Joe Perches 2020-06-12 14:40 ` Michael Ellerman 2020-06-14 21:29 ` Pavel Machek 2020-06-15 4:21 ` Jiri Slaby 2020-06-15 6:12 ` Pavel Machek 2020-06-15 6:46 ` SeongJae Park 2020-06-15 7:00 ` Joe Perches 2020-06-15 7:39 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).