Re: [PATCH v3 1/1] scripts: Add add-maintainer.py

From: Guru Das Srinagesh <quic_gurus@quicinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Guru Das Srinagesh <quic_gurus@quicinc.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	Kees Cook <keescook@chromium.org>,
	Bjorn Andersson <andersson@kernel.org>, <robh+dt@kernel.org>,
	<krzysztof.kozlowski+dt@linaro.org>,
	Will Deacon <will@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	<quic_pkondeti@quicinc.com>, <linux-kernel@vger.kernel.org>,
	<kernel@quicinc.com>, <workflows@vger.kernel.org>,
	<tools@linux.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-arm-msm@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-pm@vger.kernel.org>
Subject: Re: [PATCH v3 1/1] scripts: Add add-maintainer.py
Date: Mon, 28 Aug 2023 10:56:29 -0700	[thread overview]
Message-ID: <20230828175629.GC23466@quicinc.com> (raw)
In-Reply-To: <2efba6b3-2399-9deb-d0ce-78f7b5e12f30@linaro.org>

On Aug 28 2023 10:21, Krzysztof Kozlowski wrote:
> On 26/08/2023 10:07, Guru Das Srinagesh wrote:
> > This script runs get_maintainer.py on a given patch file (or multiple
> > patch files) and adds its output to the patch file in place with the
> > appropriate email headers "To: " or "Cc: " as the case may be. These new
> > headers are added after the "From: " line in the patch.
> > 
> > Currently, for a single patch, maintainers and reviewers are added as
> > "To: ", mailing lists and all other roles are added as "Cc: ".
> > 
> > For a series of patches, however, a set-union scheme is employed in
> > order to solve the all-too-common problem of ending up sending only
> > subsets of a patch series to some lists, which results in important
> > pieces of context such as the cover letter (or other patches in the
> > series) being dropped from those lists. This scheme is as follows:
> > 
> > - Create set-union of all maintainers and reviewers from all patches and
> >   use this to do the following per patch:
> >   - add only that specific patch's maintainers and reviewers as "To: "
> >   - add the other maintainers and reviewers from the other patches as "Cc: "
> > 
> > - Create set-union of all mailing lists corresponding to all patches and
> >   add this to all patches as "Cc: "
> > 
> > - Create set-union of all other roles corresponding to all patches and
> >   add this to all patches as "Cc: "
> > 
> > Please note that patch files that don't have any "Maintainer"s or
> > "Reviewers" explicitly listed in their `get_maintainer.pl` output will
> 
> So before you will ignoring the reviewers, right? One more reason to not
> get it right...

In v2, Reviewers were added as "Cc:" whereas here in v3 they are added as
"To:". Not sure where you're getting "ignoring the reviewers" from.

> > not have any "To: " entries added to them; developers are expected to
> > manually make edits to the added entries in such cases to convert some
> > "Cc: " entries to "To: " as desired.
> > 
> > The script is quiet by default (only prints errors) and its verbosity
> > can be adjusted via an optional parameter.
> > 
> > Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
> > ---
> >  MAINTAINERS               |   5 ++
> >  scripts/add-maintainer.py | 164 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 169 insertions(+)
> >  create mode 100755 scripts/add-maintainer.py
> > 
> 
> I do not see the benefits of this script. For me - it's unnecessarily
> more complicated instead of my simple bash function which makes

Your function adds mailing lists also in "To:" which is not ideal, in my view.
You've mentioned before that To or Cc doesn't matter [1] which I disagree
with: it doesn't matter, why does Cc exist as a concept at all?

[1] https://lore.kernel.org/lkml/af1eca37-9fd2-1e83-ab27-ebb51480904b@linaro.org/

Thank you.

Guru Das.