Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   SeongJae Park <sj38.park@gmail.com>
To:     Joe Perches <joe@perches.com>
Cc:     SeongJae Park <sj38.park@gmail.com>,
        =?UTF-8?q?Micha=C5=82=20Miros=C5=82aw?= <mirq-linux@rere.qmqm.pl>,
        "SeongJae Park" <sjpark@amazon.com>,
        "Andrew Morton" <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, apw@canonical.com,
        colin.king@canonical.com, jslaby@suse.cz, pavel@ucw.cz,
        "SeongJae Park" <sjpark@amazon.de>
Subject: Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking
Date:   Sun, 26 Jul 2020 20:07:48 +0200
Message-Id: <20200726180748.29924-1-sj38.park@gmail.com>
In-Reply-To: <5f5679b4c76bf0e16064f3a45319bc35938ad96b.camel@perches.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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