On 2/11/25 11:48, Geert Uytterhoeven wrote:
> Hi Uwe,
> 
> On Tue, 11 Feb 2025 at 11:32, Uwe Kleine-König
> <[email protected]> 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:
>> >
>> > ...
>> > [email protected] (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 <[email protected]>
>>
>> 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 <[email protected]>" $(: maintainer:OPEN FIRMWARE AND 
>> FLATTENED DEVICE TREE BINDINGS) \
>>         -t "Krzysztof Kozlowski <[email protected]>" $(: maintainer:OPEN 
>> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
>>         -t "Conor Dooley <[email protected]>" $(: maintainer:OPEN FIRMWARE 
>> AND FLATTENED DEVICE TREE BINDINGS) \
>>         -t "Neil Armstrong <[email protected]>" $(: 
>> maintainer:ARM/Amlogic Meson SoC support) \
>>         -t "Kevin Hilman <[email protected]>" $(: maintainer:ARM/Amlogic 
>> Meson SoC support) \
>>         -c "Jerome Brunet <[email protected]>" $(: reviewer:ARM/Amlogic 
>> Meson SoC support) \
>>         -c "Martin Blumenstingl <[email protected]>" $(: 
>> reviewer:ARM/Amlogic Meson SoC support) \
>>         -t "Liam Girdwood <[email protected]>" $(: supporter:SOUND - SOC 
>> LAYER / DYNAMIC AUDIO POWER MANAGEM...) \
>>         -t "Mark Brown <[email protected]>" $(: supporter:SOUND - SOC LAYER 
>> / DYNAMIC AUDIO POWER MANAGEM...) \
>>         -t "Jaroslav Kysela <[email protected]>" $(: maintainer:SOUND) \
>>         -t "Takashi Iwai <[email protected]>" $(: maintainer:SOUND) \
>>         -c "[email protected]" $(: open list:OPEN FIRMWARE AND 
>> FLATTENED DEVICE TREE BINDINGS) \
>>         -c "[email protected]" $(: moderated 
>> list:ARM/Amlogic Meson SoC support) \
>>         -c "[email protected]" $(: open list:ARM/Amlogic 
>> Meson SoC support) \
>>         -c "[email protected]" $(: open list) \
>>         -c "[email protected]" $(: 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 <[email protected]> (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.
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
> 


Reply via email to