Message-ID: <CAMuHMdXNaNPiVXMrqf4YqM-PRC6ZrTgmHtOP1S9PPCP2HqvGWA@mail.gmail.com>
Date: Tue, 11 Feb 2025 16:19:53 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Uwe Kleine-König <u.kleine-koenig@...libre.com>, 
	Joe Perches <joe@...ches.com>, Andrew Morton <akpm@...ux-foundation.org>, workflows@...r.kernel.org, 
	"Theodore Ts'o" <tytso@....edu>, "Bryan O'Donoghue" <bryan.odonoghue@...aro.org>, 
	Thorsten Leemhuis <linux@...mhuis.info>, Kees Cook <kees@...nel.org>, linux-kernel@...r.kernel.org, 
	regressions@...ts.linux.dev
Subject: Re: [PATCH v2 1/2] get_maintainer: add --substatus for reporting
 subsystem status

Hi Vlastimil,

On Tue, 11 Feb 2025 at 15:58, Vlastimil Babka <vbabka@...e.cz> wrote:
> On 2/11/25 11:48, Geert Uytterhoeven wrote:
> > On Tue, 11 Feb 2025 at 11:32, Uwe Kleine-König
> > <u.kleine-koenig@...libre.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@...r.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@...e.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@...nel.org>" $(: maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
> >>         -t "Krzysztof Kozlowski <krzk+dt@...nel.org>" $(: maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
> >>         -t "Conor Dooley <conor+dt@...nel.org>" $(: maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
> >>         -t "Neil Armstrong <neil.armstrong@...aro.org>" $(: maintainer:ARM/Amlogic Meson SoC support) \
> >>         -t "Kevin Hilman <khilman@...libre.com>" $(: maintainer:ARM/Amlogic Meson SoC support) \
> >>         -c "Jerome Brunet <jbrunet@...libre.com>" $(: reviewer:ARM/Amlogic Meson SoC support) \
> >>         -c "Martin Blumenstingl <martin.blumenstingl@...glemail.com>" $(: reviewer:ARM/Amlogic Meson SoC support) \
> >>         -t "Liam Girdwood <lgirdwood@...il.com>" $(: supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) \
> >>         -t "Mark Brown <broonie@...nel.org>" $(: supporter:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...) \
> >>         -t "Jaroslav Kysela <perex@...ex.cz>" $(: maintainer:SOUND) \
> >>         -t "Takashi Iwai <tiwai@...e.com>" $(: maintainer:SOUND) \
> >>         -c "devicetree@...r.kernel.org" $(: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) \
> >>         -c "linux-arm-kernel@...ts.infradead.org" $(: moderated list:ARM/Amlogic Meson SoC support) \
> >>         -c "linux-amlogic@...ts.infradead.org" $(: open list:ARM/Amlogic Meson SoC support) \
> >>         -c "linux-kernel@...r.kernel.org" $(: open list) \
> >>         -c "linux-sound@...r.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@...mple.com> (maintainer:SUBSYSTEM [supported])
>
> But it seemed noisy to repeat that on every line involving the subsystem.

Yeah, it could be considered noisy... (more below)

> When you say comment, what kind of separation for the comment would work
> regardless of what's used for postprocessing?

I don't mind much. Perhaps just a comma?

> > 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...

I still seem to miss the real story behind this patch (so perhaps
that's why I would consider all of it noisy ;-). When I create a patch,
what am I gonna do with this extra information?
E.g. decide not to send the patch, because the driver is orphaned?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists