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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 28 Aug 2023 19:59:54 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     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 28/08/2023 19:56, Guru Das Srinagesh wrote:
> 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@...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
> 
> 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?

To/Cc does not matter when sending new patch, because maintainers know
they are maintainers of which parts. I know what I handle.

To/Cc still makes sense in other cases, when for example you ping
someone asking for reviews. It also makes much more sense in all
corpo-worlds where such distinction is obvious. We are not a corpo-world
here.


Best regards,
Krzysztof