* [RFC PATCH] get_maintainer: decouple subsystem status from maintainer role
@ 2024-12-13 11:29 Vlastimil Babka
2024-12-18 5:48 ` Kees Cook
0 siblings, 1 reply; 4+ messages in thread
From: Vlastimil Babka @ 2024-12-13 11:29 UTC (permalink / raw)
To: Joe Perches, workflows
Cc: linux-kernel, Vlastimil Babka, Theodore Ts'o,
Bryan O'Donoghue, Thorsten Leemhuis
The script currently uses the subystem's status (S: field) to change how
maintainers are reported. One prominent example is when the status is
Supported, the maintainers are reported as "(supporter:SUBSYSTEM)".
This is misleading, as the Supported status defined as "Someone is
actually paid to look after this." may not in fact apply to everyone
listed as a maintainer, but only to some of them.
It has also been confusing people to what "supporter" means and has
required updates to the documentation [1].
Thus stop applying the subsystem status to change "maintainer:" to
anything else, as maintainers are maintainers. Instead, if the subsystem
status is not the most common one (Maintained), indicate it as part of
the subsystem name. So for example, instead of "(supporter:SUBSYSTEM)"
report "(maintainer:SUBSYSTEM [supported])".
[1] https://lore.kernel.org/all/20221006162413.858527-1-bryan.odonoghue@linaro.org/
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: "Bryan O'Donoghue" <bryan.odonoghue@linaro.org>
Cc: Thorsten Leemhuis <linux@leemhuis.info>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
I have been confused myself in the past seeing "supporter" and have seen
somebody recently wondering what it means as well.
I have read the threads from 2022 that in the end resulted in adjusting
documentation only [1]. I very much agree with Ted's points about taking
the subsystem status and applying it to all maintainers being wrong [2].
The attempt to modify get_maintainer output was retracted after Joe
objected that the status becomes not reported at all [3]. This RFC
attempts to address that by reporting the status (unless it's the most
common one) as part of the subsystem.
The patch is not perfect, as with this approach, the logical thing would
be to do the same also for reviewers and mailing lists. In fact,
subsystems with a status of Orphan typically only have some catch-all
mailing list and no maintainers, so the "(orphan minder:SUBSYSTEM)"
would never be currently reported by checkpatch. It would be thus
logical to report the status in the same way for lists (and reviewers).
But I didn't attempt a full implementation as I'm not fluent in Perl and
would like to see if we can get a consensus first. If we do, I don't
insist in this particular "SUBSYSTEM [status]" syntax nor on
implementing the full solution myself - I would be happy if somebody
else did. My main point is that maintainer is a maintainer and the
subsystem status should be indicated for the subsystem, not for the
maintainer.
[1] https://lore.kernel.org/all/20221006162413.858527-1-bryan.odonoghue@linaro.org/
[2] https://lore.kernel.org/all/Yzen4X1Na0MKXHs9@mit.edu/
[3] https://lore.kernel.org/all/30776fe75061951777da8fa6618ae89bea7a8ce4.camel@perches.com/
scripts/get_maintainer.pl | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 5ac02e198737..a2f578f2d93b 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -1286,6 +1286,7 @@ sub get_maintainer_role {
my $role = "unknown";
my $subsystem = get_subsystem_name($index);
+ my $substatus = "";
for ($i = $start + 1; $i < $end; $i++) {
my $tv = $typevalue[$i];
@@ -1299,21 +1300,10 @@ sub get_maintainer_role {
}
$role = lc($role);
- if ($role eq "supported") {
- $role = "supporter";
- } elsif ($role eq "maintained") {
- $role = "maintainer";
- } elsif ($role eq "odd fixes") {
- $role = "odd fixer";
- } elsif ($role eq "orphan") {
- $role = "orphan minder";
- } elsif ($role eq "obsolete") {
- $role = "obsolete minder";
- } elsif ($role eq "buried alive in reporters") {
- $role = "chief penguin";
- }
-
- return $role . ":" . $subsystem;
+ if ($role ne "maintained") {
+ $substatus = " [" . $role . "]";
+ }
+ return "maintainer:" . $subsystem . $substatus;
}
sub get_list_role {
--
2.47.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] get_maintainer: decouple subsystem status from maintainer role
2024-12-13 11:29 [RFC PATCH] get_maintainer: decouple subsystem status from maintainer role Vlastimil Babka
@ 2024-12-18 5:48 ` Kees Cook
2025-01-06 18:21 ` Thorsten Leemhuis
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2024-12-18 5:48 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Joe Perches, workflows, linux-kernel, Theodore Ts'o,
Bryan O'Donoghue, Thorsten Leemhuis
On Fri, Dec 13, 2024 at 12:29:22PM +0100, Vlastimil Babka wrote:
> The script currently uses the subystem's status (S: field) to change how
> maintainers are reported. One prominent example is when the status is
> Supported, the maintainers are reported as "(supporter:SUBSYSTEM)".
>
> This is misleading, as the Supported status defined as "Someone is
> actually paid to look after this." may not in fact apply to everyone
> listed as a maintainer, but only to some of them.
>
> It has also been confusing people to what "supporter" means and has
> required updates to the documentation [1].
>
> Thus stop applying the subsystem status to change "maintainer:" to
> anything else, as maintainers are maintainers. Instead, if the subsystem
> status is not the most common one (Maintained), indicate it as part of
> the subsystem name. So for example, instead of "(supporter:SUBSYSTEM)"
> report "(maintainer:SUBSYSTEM [supported])".
>
> [1] https://lore.kernel.org/all/20221006162413.858527-1-bryan.odonoghue@linaro.org/
>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: "Bryan O'Donoghue" <bryan.odonoghue@linaro.org>
> Cc: Thorsten Leemhuis <linux@leemhuis.info>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Kees Cook <kees@kernel.org>
> ---
> I have been confused myself in the past seeing "supporter" and have seen
> somebody recently wondering what it means as well.
>
> I have read the threads from 2022 that in the end resulted in adjusting
> documentation only [1]. I very much agree with Ted's points about taking
> the subsystem status and applying it to all maintainers being wrong [2].
>
> The attempt to modify get_maintainer output was retracted after Joe
> objected that the status becomes not reported at all [3]. This RFC
> attempts to address that by reporting the status (unless it's the most
> common one) as part of the subsystem.
>
> The patch is not perfect, as with this approach, the logical thing would
> be to do the same also for reviewers and mailing lists. In fact,
> subsystems with a status of Orphan typically only have some catch-all
> mailing list and no maintainers, so the "(orphan minder:SUBSYSTEM)"
> would never be currently reported by checkpatch. It would be thus
> logical to report the status in the same way for lists (and reviewers).
>
> But I didn't attempt a full implementation as I'm not fluent in Perl and
> would like to see if we can get a consensus first. If we do, I don't
> insist in this particular "SUBSYSTEM [status]" syntax nor on
> implementing the full solution myself - I would be happy if somebody
> else did. My main point is that maintainer is a maintainer and the
> subsystem status should be indicated for the subsystem, not for the
> maintainer.
>
> [1] https://lore.kernel.org/all/20221006162413.858527-1-bryan.odonoghue@linaro.org/
> [2] https://lore.kernel.org/all/Yzen4X1Na0MKXHs9@mit.edu/
> [3] https://lore.kernel.org/all/30776fe75061951777da8fa6618ae89bea7a8ce4.camel@perches.com/
Do we want to change "Supported" to "Funded" to help clear up the
meaning? (But yes, I agree, that the subsystem status should be applied
to the subsystem, not the individual contacts.)
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] get_maintainer: decouple subsystem status from maintainer role
2024-12-18 5:48 ` Kees Cook
@ 2025-01-06 18:21 ` Thorsten Leemhuis
2025-01-13 14:55 ` Vlastimil Babka
0 siblings, 1 reply; 4+ messages in thread
From: Thorsten Leemhuis @ 2025-01-06 18:21 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Joe Perches, workflows, linux-kernel, Theodore Ts'o,
Bryan O'Donoghue, Kees Cook
Lo! From the "better reply late than never" department:
Thx for picking this up again, much appreciated!
On 18.12.24 06:48, Kees Cook wrote:
> On Fri, Dec 13, 2024 at 12:29:22PM +0100, Vlastimil Babka wrote:
>> The script currently uses the subystem's status (S: field) to change how
>> maintainers are reported. One prominent example is when the status is
>> Supported, the maintainers are reported as "(supporter:SUBSYSTEM)".
>>
>> This is misleading, as the Supported status defined as "Someone is
>> actually paid to look after this." may not in fact apply to everyone
>> listed as a maintainer, but only to some of them.
>>
>> It has also been confusing people to what "supporter" means and has
>> required updates to the documentation [1].
>>
>> Thus stop applying the subsystem status to change "maintainer:" to
>> anything else, as maintainers are maintainers. Instead, if the subsystem
>> status is not the most common one (Maintained), indicate it as part of
>> the subsystem name. So for example, instead of "(supporter:SUBSYSTEM)"
>> report "(maintainer:SUBSYSTEM [supported])".
As Kees mentioned: "funded" might be better. Or is there even a better
word for this? "backed"? "subsidized"?
When I read this for the first time I thought "it would be better to
keep the two aspects closer together, e.g.
"(maintainer[supported]:SUBSYSTEM)". But then I read... (continue below)
>> [1] https://lore.kernel.org/all/20221006162413.858527-1-bryan.odonoghue@linaro.org/
> [...]
>> ---
>> I have been confused myself in the past seeing "supporter" and have seen
>> somebody recently wondering what it means as well.
>>
>> I have read the threads from 2022 that in the end resulted in adjusting
>> documentation only [1]. I very much agree with Ted's points about taking
>> the subsystem status and applying it to all maintainers being wrong [2].
>>
>> The attempt to modify get_maintainer output was retracted after Joe
>> objected that the status becomes not reported at all [3]. This RFC
>> attempts to address that by reporting the status (unless it's the most
>> common one) as part of the subsystem.
>>
>> The patch is not perfect, as with this approach, the logical thing would
>> be to do the same also for reviewers and mailing lists. In fact,
>> subsystems with a status of Orphan typically only have some catch-all
>> mailing list and no maintainers, so the "(orphan minder:SUBSYSTEM)"
>> would never be currently reported by checkpatch. It would be thus
>> logical to report the status in the same way for lists (and reviewers).
>>
>> But I didn't attempt a full implementation as I'm not fluent in Perl and
>> would like to see if we can get a consensus first. If we do, I don't
>> insist in this particular "SUBSYSTEM [status]" syntax nor on
>> implementing the full solution myself - I would be happy if somebody
>> else did. My main point is that maintainer is a maintainer and the
>> subsystem status should be indicated for the subsystem, not for the
>> maintainer.
>>
>> [1] https://lore.kernel.org/all/20221006162413.858527-1-bryan.odonoghue@linaro.org/
>> [2] https://lore.kernel.org/all/Yzen4X1Na0MKXHs9@mit.edu/
>> [3] https://lore.kernel.org/all/30776fe75061951777da8fa6618ae89bea7a8ce4.camel@perches.com/
>
> Do we want to change "Supported" to "Funded" to help clear up the
> meaning? (But yes, I agree, that the subsystem status should be applied
> to the subsystem, not the individual contacts.)
...this and thought: well, the current format of MAINTAINERS applies the
status to all maintainers, even is some of them are funded while others
are not. Changing this would likely require bigger changes. But I'm
unsure if that is really worth it. Guess not, because it's likely a rare
case. So I guess the format you chose is the best for now.
Thx again for doing this, I like it.
Ciao, Thorsten
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] get_maintainer: decouple subsystem status from maintainer role
2025-01-06 18:21 ` Thorsten Leemhuis
@ 2025-01-13 14:55 ` Vlastimil Babka
0 siblings, 0 replies; 4+ messages in thread
From: Vlastimil Babka @ 2025-01-13 14:55 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Joe Perches, workflows, linux-kernel, Theodore Ts'o,
Bryan O'Donoghue, Kees Cook
On 1/6/25 19:21, Thorsten Leemhuis wrote:
> Lo! From the "better reply late than never" department:
>
> Thx for picking this up again, much appreciated!
Thank you both for the support :)
> On 18.12.24 06:48, Kees Cook wrote:
>> On Fri, Dec 13, 2024 at 12:29:22PM +0100, Vlastimil Babka wrote:
>>> The script currently uses the subystem's status (S: field) to change how
>>> maintainers are reported. One prominent example is when the status is
>>> Supported, the maintainers are reported as "(supporter:SUBSYSTEM)".
>>>
>>> This is misleading, as the Supported status defined as "Someone is
>>> actually paid to look after this." may not in fact apply to everyone
>>> listed as a maintainer, but only to some of them.
>>>
>>> It has also been confusing people to what "supporter" means and has
>>> required updates to the documentation [1].
>>>
>>> Thus stop applying the subsystem status to change "maintainer:" to
>>> anything else, as maintainers are maintainers. Instead, if the subsystem
>>> status is not the most common one (Maintained), indicate it as part of
>>> the subsystem name. So for example, instead of "(supporter:SUBSYSTEM)"
>>> report "(maintainer:SUBSYSTEM [supported])".
>
> As Kees mentioned: "funded" might be better. Or is there even a better
> word for this? "backed"? "subsidized"?
Sure we can try find a better word, but in that case it should be applied at
the source, i.e. in MAINTAINERS, not in the script. After my change, the
script stops trying to reinterpret the exact wording from MAINTAINERS anyway.
> When I read this for the first time I thought "it would be better to
> keep the two aspects closer together, e.g.
> "(maintainer[supported]:SUBSYSTEM)". But then I read... (continue below)
>
<snip>
>> Do we want to change "Supported" to "Funded" to help clear up the
>> meaning? (But yes, I agree, that the subsystem status should be applied
>> to the subsystem, not the individual contacts.)
>
> ...this and thought: well, the current format of MAINTAINERS applies the
> status to all maintainers,
AFAIU it applies it to subsystems. It defines status e.g. as:
"Supported: Someone is actually paid to look after this."
See how it says "someone", not "everyone with a M: entry". It's only the
script that's reinterpreting this definition (in a misleading way).
> even is some of them are funded while others
> are not. Changing this would likely require bigger changes. But I'm
> unsure if that is really worth it. Guess not, because it's likely a rare
> case. So I guess the format you chose is the best for now.
Agree. It should be enough to indicate to the submitter that "someone is
paid" and thus one of the M: people CC'd is hopefully more likely to respond
than if nobody is paid.
But I guess the biggest utility here is rather on the other side, i.e.
indicating "Orphan" or "Obsolete" can be a heads up wrt expectations of a reply.
> Thx again for doing this, I like it.
Thanks, will resubmit then as the original timing was quite close to
holidays. Will also look at indicating the subsystem status for reviewers
and mailing lists too (as I said, "Orphan" will likely not have an M: entry
which would carry the status in the first place), since nobody fluent in
Perl has volunteered :)
> Ciao, Thorsten
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-01-13 14:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-13 11:29 [RFC PATCH] get_maintainer: decouple subsystem status from maintainer role Vlastimil Babka
2024-12-18 5:48 ` Kees Cook
2025-01-06 18:21 ` Thorsten Leemhuis
2025-01-13 14:55 ` Vlastimil Babka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox