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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Sep 2023 10:21:42 +0530
From:   Pavan Kondeti <quic_pkondeti@...cinc.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC:     Guru Das Srinagesh <quic_gurus@...cinc.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        Nicolas Schier <nicolas@...sle.eu>,
        Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>,
        Bjorn Andersson <andersson@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, Will Deacon <will@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <quic_pkondeti@...cinc.com>, <linux-kernel@...r.kernel.org>,
        <kernel@...cinc.com>, <workflows@...r.kernel.org>,
        <tools@...ux.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v3 1/1] scripts: Add add-maintainer.py

On Mon, Aug 28, 2023 at 10:21:32AM +0200, 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...
> 
> > 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@...cinc.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
> everything one command less than here.
> One more thing to maintain.

Thanks for your bash script. I slightly modified it to keep maintainers
in To and rest in Cc. 

git send-email --dry-run --to="$(scripts/get_maintainer.pl --no-multiline --separator=, --no-r --no-l --no-git --no-roles --no-rolestats --no-git-fallback *.patch)" --cc="$(scripts/get_maintainer.pl --no-multiline --separator=, --no-m --no-git --no-roles --no-rolestats --no-git-fallback *.patch)" *.patch

> 
> I don't see the benefits of this for newcomers, either. They should use
> b4. b4 can do much, much more, so anyone creating his workflow should
> start from b4, not from this script.

The ROI from b4 is huge. Totally agree that one should definitely consider b4 for
kernel patch submissions. I really liked b4 appending a unique "change-id"
across patch-versions. This is the single most feature I miss out from gerrit where
all revisions of a patch are glued with a common change-id. If everyone uses, a common
change-id for all versions of series, it is super easy to dig into archives.

Thanks,
Pavan

Thanks,
Pavan