Re: [PATCH v2 1/2] get_maintainer: add --substatus for reporting subsystem status

[Vlastimil Babka <vbabka@suse.cz>]

On 2/11/25 11:59, Vlastimil Babka wrote:
> On 2/11/25 11:48, Geert Uytterhoeven wrote:
>> Hi Uwe,
>> 
>> On Tue, 11 Feb 2025 at 11:32, Uwe Kleine-König
>> <u.kleine-koenig@baylibre.com> wrote:
>>> On Mon, Feb 03, 2025 at 12:13:16PM +0100, Vlastimil Babka wrote:
>>> > The subsystem status is currently reported with --role(stats) by
>>> > adjusting the maintainer role for any status different from Maintained.
>>> > This has two downsides:
>>> >
>>> > - if a subsystem has only reviewers or mailing lists and no maintainers,
>>> >   the status is not reported (i.e. typically, Orphan subsystems have no
>>> >   maintainers)
>>> >
>>> > - the Supported status means that someone is paid for maintaining, but
>>> >   it is reported as "supporter" for all the maintainers, which can be
>>> >   incorrect. People have been also confused about what "supporter"
>>> >   means.
>>> >
>>> > This patch introduces a new --substatus option and functionality aimed
>>> > to report the subsystem status separately, without adjusting the
>>> > reported maintainer role. After the e-mails are output, the status of
>>> > subsystems will follow, for example:
>>> >
>>> > ...
>>> > linux-kernel@vger.kernel.org (open list:LIBRARY CODE)
>>> > LIBRARY CODE status: Supported
>>> >
>>> > In order to allow replacing the role rewriting seamlessly, the new
>>> > option works as follows:
>>> >
>>> > - it is automatically enabled when --email and --role are enabled
>>> >   (the defaults include --email and --rolestats which implies --role)
>>> >
>>> > - usages with --norolestats e.g. for git's --cc-cmd will thus need no
>>> >   adjustments
>>> >
>>> > - the most common Maintained status is not reported at all, to reduce
>>> >   unnecessary noise
>>> >
>>> > - THE REST catch-all section (contains lkml) status is not reported
>>> >
>>> > - the existing --subsystem and --status options are unaffected so their
>>> >   users will need no adjustments
>>> >
>>> > Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>>>
>>> This patch is in next as c1565b6f7b53ea1ea3e757538832e12d7d13d949. It
>>> breaks one of my scripts that I use to semi-automatically determine
>>> recipents for patch series.
>>>
>>> It works as follows:
>>>
>>>         $ batch-add-recipents audin-patch-v1/0001-ASoC-meson-HACK-let-AIU-export-its-clocks-through-cl.patch
>>>         #!/bin/sh
>>>
>>>         addrecipent \
>>>         -t "Rob Herring <robh@kernel.org>" $(: maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
>>>         -t "Krzysztof Kozlowski <krzk+dt@kernel.org>" $(: maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
>>>         -t "Conor Dooley <conor+dt@kernel.org>" $(: maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
>>>         -t "Neil Armstrong <neil.armstrong@linaro.org>" $(: maintainer:ARM/Amlogic Meson SoC support) \
>>>         -t "Kevin Hilman <khilman@baylibre.com>" $(: maintainer:ARM/Amlogic Meson SoC support) \
>>>         -c "Jerome Brunet <jbrunet@baylibre.com>" $(: reviewer:ARM/Amlogic Meson SoC support) \
>>>         -c "Martin Blumenstingl <martin.blumenstingl@googlemail.com>" $(: reviewer:ARM/Amlogic Meson SoC support) \
>>>         -t "Liam Girdwood <lgirdwood@gmail.com>" $(: supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) \
>>>         -t "Mark Brown <broonie@kernel.org>" $(: supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) \
>>>         -t "Jaroslav Kysela <perex@perex.cz>" $(: maintainer:SOUND) \
>>>         -t "Takashi Iwai <tiwai@suse.com>" $(: maintainer:SOUND) \
>>>         -c "devicetree@vger.kernel.org" $(: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
>>>         -c "linux-arm-kernel@lists.infradead.org" $(: moderated list:ARM/Amlogic Meson SoC support) \
>>>         -c "linux-amlogic@lists.infradead.org" $(: open list:ARM/Amlogic Meson SoC support) \
>>>         -c "linux-kernel@vger.kernel.org" $(: open list) \
>>>         -c "linux-sound@vger.kernel.org" $(: open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) \
>>>         audin-patch-v1/0001-ASoC-meson-HACK-let-AIU-export-its-clocks-through-cl.patch
>> 
>> Hey, that looks familiar ;-)
>> 
>>> the output is usually redirected to a file that I edit before running
>>> it. The additional line in the output of
>>>
>>>         scripts/get_maintainer.pl audin-patch-v1/0001-ASoC-meson-HACK-let-AIU-export-its-clocks-through-cl.patch
>>>
>>> with your change breaks that script.
>> 
>> You forgot to list the additional output?
>> 
>> I gave it a try with my script, and with one of my own patches.
>> Example additional output is:
>> 
>>      --cc "ARM/Microchip" $(: AT91] SoC support status: Supported \
>>     --cc "QAT DRIVER status: Supported \
>>     --cc "ARM/ASPEED MACHINE SUPPORT status: Supported \
>>     --cc "MELEXIS MLX90614 DRIVER status: Supported \
>>     --cc "ARM/NUVOTON MA35 ARCHITECTURE status: Supported \
>>     --cc "ARM/RISC-V/RENESAS ARCHITECTURE status: Supported \
>> 
>> Iff this extra output is good to have, why not include it in the comment
>> next to the existing entries with the email addresses, so it will be
>> handled automatically by all scripting on top?
> 
> I've tried to do that in v1 in the form of reporting e.g. as
> John Doe <jd@example.com> (maintainer:SUBSYSTEM [supported])
> 
> But it seemed noisy to repeat that on every line involving the subsystem.
> 
> When you say comment, what kind of separation for the comment would work
> regardless of what's used for postprocessing?
> 
>> Now, as both Uwe and I edit our generated scripts before running them,
>> we can delete the unwanted lines, but it's more work...
>> Thanks!
> 
> I guess technically your scripts could detect first if --no-substatus is
> supported by grepping --help or testing if passing the option results in an
> error? But yeah it's not ideal, looks like I've hit the limits of automagic
> heuristics here.

Hm maybe I could add "-t STDOUT" to the heuristics?

> Or we make it fully opt-in but then most non-scripting users will not learn
> the status at all because it won't occur to them to enable it...
> 
>> Gr{oetje,eeting}s,
>> 
>>                         Geert
>> 
>