2022-09-28 01:07:39

by Bryan O'Donoghue

[permalink] [raw]
Subject: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

Recently when submitting a yaml change I found that I had omitted the
maintainer whose tree the change needed to go through.

The reason for that is the path in MAINTAINERS is marked as Supported not
Maintained. Reading MAINTAINERS we see quote:

Supported: Someone is actually paid to look after this.
Maintained: Someone actually looks after it.

The current submitting-patches.rst only says to mail maintainers though not
supporters. When we run scripts/get_maintainer.pl anybody who is denoted a
paid maintainer will appear as a supporter.

Let's add some text to the submitting-patches.rst to indicate that
supporters should similarly be mailed so that you can't do as I did and
mail every maintainer get_maintainer.pl tells you to, without actually
mailing the one supporter you need to.

Signed-off-by: Bryan O'Donoghue <[email protected]>
---
Documentation/process/submitting-patches.rst | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index be49d8f2601b4..5f97379da41da 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -227,9 +227,11 @@ You should always copy the appropriate subsystem maintainer(s) on any patch
to code that they maintain; look through the MAINTAINERS file and the
source code revision history to see who those maintainers are. The
script scripts/get_maintainer.pl can be very useful at this step (pass paths to
-your patches as arguments to scripts/get_maintainer.pl). If you cannot find a
-maintainer for the subsystem you are working on, Andrew Morton
-([email protected]) serves as a maintainer of last resort.
+your patches as arguments to scripts/get_maintainer.pl). You should mail
+everyone who appears as "maintainer" or "supporter" in the
+scripts/get_maintainer.pl output. If you cannot find a maintainer for the
+subsystem you are working on, Andrew Morton ([email protected]) serves
+as a maintainer of last resort.

You should also normally choose at least one mailing list to receive a copy
of your patch set. [email protected] should be used by default
--
2.37.3


2022-09-28 04:56:40

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

On 28.09.22 02:30, Bryan O'Donoghue wrote:
> Recently when submitting a yaml change I found that I had omitted the
> maintainer whose tree the change needed to go through.
>
> The reason for that is the path in MAINTAINERS is marked as Supported not
> Maintained. Reading MAINTAINERS we see quote:
>
> Supported: Someone is actually paid to look after this.
> Maintained: Someone actually looks after it.
>
> The current submitting-patches.rst only says to mail maintainers though not
> supporters. When we run scripts/get_maintainer.pl anybody who is denoted a
> paid maintainer will appear as a supporter.
>
> Let's add some text to the submitting-patches.rst to indicate that
> supporters should similarly be mailed so that you can't do as I did and
> mail every maintainer get_maintainer.pl tells you to, without actually
> mailing the one supporter you need to.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>

Looks good to me, but while at it, one quick question: Would
Documentation/process/5.Posting.rst (which sadly covers exactly the same
topic) benefit from a similar clarification, even if it doesn't mention
get_maintainers explicitly?

Which leads to two other question: Are there any other places that might
benefit from such a clarification? Or would it be even make sense to
change the format of MAINTAINERS to avoid the problem in the first
place? Maybe something like "Maintained(v)" (Someone volunteered to look
after it in spare hours.) and "Maintained(p)" (Someone is actually paid
to look after this.). Ahh, no, that doesn't look good. But you get the idea.

> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index be49d8f2601b4..5f97379da41da 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -227,9 +227,11 @@ You should always copy the appropriate subsystem maintainer(s) on any patch
> to code that they maintain; look through the MAINTAINERS file and the
> source code revision history to see who those maintainers are. The
> script scripts/get_maintainer.pl can be very useful at this step (pass paths to
> -your patches as arguments to scripts/get_maintainer.pl). If you cannot find a
> -maintainer for the subsystem you are working on, Andrew Morton
> -([email protected]) serves as a maintainer of last resort.
> +your patches as arguments to scripts/get_maintainer.pl). You should mail
> +everyone who appears as "maintainer" or "supporter" in the
> +scripts/get_maintainer.pl output.

Side note and bikeshedding: Not sure, I wonder if the 'in the
scripts/get_maintainer.pl output' can be dropped to make things shorter.
Or maybe even shorter along the lines of 'Mail everyone listed as
"maintainer" or "supporter"'?

Whatever, not that important.

> If you cannot find a maintainer for the
> +subsystem you are working on, Andrew Morton ([email protected]) serves
> +as a maintainer of last resort.
>
> You should also normally choose at least one mailing list to receive a copy
> of your patch set. [email protected] should be used by default

Ciao, Thorsten

2022-09-28 07:42:01

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

On 28/09/2022 02:30, Bryan O'Donoghue wrote:
> Recently when submitting a yaml change I found that I had omitted the
> maintainer whose tree the change needed to go through.

Thank you for your patch. There is something to discuss/improve.

> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index be49d8f2601b4..5f97379da41da 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -227,9 +227,11 @@ You should always copy the appropriate subsystem maintainer(s) on any patch
> to code that they maintain; look through the MAINTAINERS file and the
> source code revision history to see who those maintainers are. The
> script scripts/get_maintainer.pl can be very useful at this step (pass paths to
> -your patches as arguments to scripts/get_maintainer.pl). If you cannot find a
> -maintainer for the subsystem you are working on, Andrew Morton
> -([email protected]) serves as a maintainer of last resort.
> +your patches as arguments to scripts/get_maintainer.pl). You should mail
> +everyone who appears as "maintainer" or "supporter" in the
> +scripts/get_maintainer.pl output. If you cannot find a maintainer for the
> +subsystem you are working on, Andrew Morton ([email protected]) serves
> +as a maintainer of last resort.

That's still a bit misleading, because you should also send the patch to
reviewers and mailing lists. :)

Best regards,
Krzysztof

2022-09-28 12:22:11

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

On 28/09/2022 07:53, Krzysztof Kozlowski wrote:
> That's still a bit misleading, because you should also send the patch to
> reviewers and mailing lists. ????

I can add something along the lines of "and whatever subsystem mailing
list is called returned by the script"

and leave it to the discretion of the patch sender to determine if they
want to include LKML or just go straight to the subsystem.

---
bod

2022-09-28 12:39:34

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

On 28.09.22 13:48, Bryan O'Donoghue wrote:
> On 28/09/2022 05:34, Thorsten Leemhuis wrote:
>> On 28.09.22 02:30, Bryan O'Donoghue wrote:
>>> Recently when submitting a yaml change I found that I had omitted the
>>> maintainer whose tree the change needed to go through.
>>>
>>> The reason for that is the path in MAINTAINERS is marked as Supported
>>> not
>>> Maintained. Reading MAINTAINERS we see quote:
>>>
>>>             Supported:   Someone is actually paid to look after this.
>>>             Maintained:  Someone actually looks after it.
>>>
>>> The current submitting-patches.rst only says to mail maintainers
>>> though not
>>> supporters. When we run scripts/get_maintainer.pl anybody who is
>>> denoted a
>>> paid maintainer will appear as a supporter.
>>>
>>> Let's add some text to the submitting-patches.rst to indicate that
>>> supporters should similarly be mailed so that you can't do as I did and
>>> mail every maintainer get_maintainer.pl tells you to, without actually
>>> mailing the one supporter you need to.
> [...]
>> Which leads to two other question: Are there any other places that might
>> benefit from such a clarification? Or would it be even make sense to
>> change the format of MAINTAINERS to avoid the problem in the first
>> place? Maybe something like "Maintained(v)" (Someone volunteered to look
>> after it in spare hours.) and "Maintained(p)" (Someone is actually paid
>> to look after this.). Ahh, no, that doesn't look good. But you get the
>> idea.
>
> We could update get_maintainer to print out something else
> such as

I really like the idea of just changing get_maintainer, but also...

> scripts/get_maintainer.pl
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>
> Andy Gross <[email protected]> (maintainer:ARM/QUALCOMM SUPPORT)
> Bjorn Andersson <[email protected]> (maintainer:ARM/QUALCOMM
> SUPPORT)
> Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM
> SUPPORT)
> Lee Jones <[email protected]> (maintainer-supporter:MULTIFUNCTION DEVICES
> (MFD))
>
> or say
>
> scripts/get_maintainer.pl
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> Andy Gross <[email protected]> (maintainer:ARM/QUALCOMM SUPPORT)
> Bjorn Andersson <[email protected]> (maintainer:ARM/QUALCOMM
> SUPPORT)
> Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM
> SUPPORT)
> Lee Jones <[email protected]> (supporting-maintainer:MULTIFUNCTION DEVICES
> (MFD))
>
> it would be less churn but, I still think we would need to update the
> documentation to be very explicit that "supporting-maintainer or
> maintainer" needs to be emailed with your patch so that sufficiently
> talented idiots such as myself, know who to mail.
>
> Although thinking about it we would be introducing yet another term
> "supporting-maintainer" to which people would say "what is that"

...agree with this.

> Feels a little less confusing to me to leave supporter as-is and just
> document expectations for patch submission better.

Hmm, how about this:

scripts/get_maintainer.pl
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
Lee Jones <[email protected]> (maintainer[supported]:MULTIFUNCTION DEVICES
(MFD))
Andy Gross <[email protected]> (maintainer[volunteer]:ARM/QUALCOMM SUPPORT)
Bjorn Andersson <[email protected]>
(maintainer[volunteer]:ARM/QUALCOMM SUPPORT)
Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM SUPPORT)

Not totally sure about this myself. And there is a risk that any such
change might break scripts that rely on the current approach used by
scripts/get_maintainer.pl :-/

Ciao, Thorsten

2022-09-28 12:40:42

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

On 28/09/2022 05:34, Thorsten Leemhuis wrote:
> On 28.09.22 02:30, Bryan O'Donoghue wrote:
>> Recently when submitting a yaml change I found that I had omitted the
>> maintainer whose tree the change needed to go through.
>>
>> The reason for that is the path in MAINTAINERS is marked as Supported not
>> Maintained. Reading MAINTAINERS we see quote:
>>
>> Supported: Someone is actually paid to look after this.
>> Maintained: Someone actually looks after it.
>>
>> The current submitting-patches.rst only says to mail maintainers though not
>> supporters. When we run scripts/get_maintainer.pl anybody who is denoted a
>> paid maintainer will appear as a supporter.
>>
>> Let's add some text to the submitting-patches.rst to indicate that
>> supporters should similarly be mailed so that you can't do as I did and
>> mail every maintainer get_maintainer.pl tells you to, without actually
>> mailing the one supporter you need to.
>>
>> Signed-off-by: Bryan O'Donoghue <[email protected]>
>
> Looks good to me, but while at it, one quick question: Would
> Documentation/process/5.Posting.rst (which sadly covers exactly the same
> topic) benefit from a similar clarification, even if it doesn't mention
> get_maintainers explicitly?

Yes.

> Which leads to two other question: Are there any other places that might
> benefit from such a clarification? Or would it be even make sense to
> change the format of MAINTAINERS to avoid the problem in the first
> place? Maybe something like "Maintained(v)" (Someone volunteered to look
> after it in spare hours.) and "Maintained(p)" (Someone is actually paid
> to look after this.). Ahh, no, that doesn't look good. But you get the idea.

We could update get_maintainer to print out something else

such as

scripts/get_maintainer.pl
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml

Andy Gross <[email protected]> (maintainer:ARM/QUALCOMM SUPPORT)
Bjorn Andersson <[email protected]> (maintainer:ARM/QUALCOMM
SUPPORT)
Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM SUPPORT)
Lee Jones <[email protected]> (maintainer-supporter:MULTIFUNCTION DEVICES
(MFD))

or say

scripts/get_maintainer.pl
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
Andy Gross <[email protected]> (maintainer:ARM/QUALCOMM SUPPORT)
Bjorn Andersson <[email protected]> (maintainer:ARM/QUALCOMM
SUPPORT)
Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM SUPPORT)
Lee Jones <[email protected]> (supporting-maintainer:MULTIFUNCTION DEVICES
(MFD))

it would be less churn but, I still think we would need to update the
documentation to be very explicit that "supporting-maintainer or
maintainer" needs to be emailed with your patch so that sufficiently
talented idiots such as myself, know who to mail.

Although thinking about it we would be introducing yet another term
"supporting-maintainer" to which people would say "what is that"

Feels a little less confusing to me to leave supporter as-is and just
document expectations for patch submission better.

>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> index be49d8f2601b4..5f97379da41da 100644
>> --- a/Documentation/process/submitting-patches.rst
>> +++ b/Documentation/process/submitting-patches.rst
>> @@ -227,9 +227,11 @@ You should always copy the appropriate subsystem maintainer(s) on any patch
>> to code that they maintain; look through the MAINTAINERS file and the
>> source code revision history to see who those maintainers are. The
>> script scripts/get_maintainer.pl can be very useful at this step (pass paths to
>> -your patches as arguments to scripts/get_maintainer.pl). If you cannot find a
>> -maintainer for the subsystem you are working on, Andrew Morton
>> -([email protected]) serves as a maintainer of last resort.
>> +your patches as arguments to scripts/get_maintainer.pl). You should mail
>> +everyone who appears as "maintainer" or "supporter" in the
>> +scripts/get_maintainer.pl output.
>
> Side note and bikeshedding: Not sure, I wonder if the 'in the
> scripts/get_maintainer.pl output' can be dropped to make things shorter.
> Or maybe even shorter along the lines of 'Mail everyone listed as
> "maintainer" or "supporter"'?
>
> Whatever, not that important.

Sure NP.

---
bod

2022-09-28 12:43:38

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH] Documentation/process: Add text to indicate supporters should be mailed

On 28/09/2022 13:02, Thorsten Leemhuis wrote:
> On 28.09.22 13:48, Bryan O'Donoghue wrote:
>> On 28/09/2022 05:34, Thorsten Leemhuis wrote:
>>> On 28.09.22 02:30, Bryan O'Donoghue wrote:
>>>> Recently when submitting a yaml change I found that I had omitted the
>>>> maintainer whose tree the change needed to go through.
>>>>
>>>> The reason for that is the path in MAINTAINERS is marked as Supported
>>>> not
>>>> Maintained. Reading MAINTAINERS we see quote:
>>>>
>>>>             Supported:   Someone is actually paid to look after this.
>>>>             Maintained:  Someone actually looks after it.
>>>>
>>>> The current submitting-patches.rst only says to mail maintainers
>>>> though not
>>>> supporters. When we run scripts/get_maintainer.pl anybody who is
>>>> denoted a
>>>> paid maintainer will appear as a supporter.
>>>>
>>>> Let's add some text to the submitting-patches.rst to indicate that
>>>> supporters should similarly be mailed so that you can't do as I did and
>>>> mail every maintainer get_maintainer.pl tells you to, without actually
>>>> mailing the one supporter you need to.
>> [...]
>>> Which leads to two other question: Are there any other places that might
>>> benefit from such a clarification? Or would it be even make sense to
>>> change the format of MAINTAINERS to avoid the problem in the first
>>> place? Maybe something like "Maintained(v)" (Someone volunteered to look
>>> after it in spare hours.) and "Maintained(p)" (Someone is actually paid
>>> to look after this.). Ahh, no, that doesn't look good. But you get the
>>> idea.
>>
>> We could update get_maintainer to print out something else
>> such as
>
> I really like the idea of just changing get_maintainer, but also...
>
>> scripts/get_maintainer.pl
>> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>>
>> Andy Gross <[email protected]> (maintainer:ARM/QUALCOMM SUPPORT)
>> Bjorn Andersson <[email protected]> (maintainer:ARM/QUALCOMM
>> SUPPORT)
>> Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM
>> SUPPORT)
>> Lee Jones <[email protected]> (maintainer-supporter:MULTIFUNCTION DEVICES
>> (MFD))
>>
>> or say
>>
>> scripts/get_maintainer.pl
>> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
>> Andy Gross <[email protected]> (maintainer:ARM/QUALCOMM SUPPORT)
>> Bjorn Andersson <[email protected]> (maintainer:ARM/QUALCOMM
>> SUPPORT)
>> Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM
>> SUPPORT)
>> Lee Jones <[email protected]> (supporting-maintainer:MULTIFUNCTION DEVICES
>> (MFD))
>>
>> it would be less churn but, I still think we would need to update the
>> documentation to be very explicit that "supporting-maintainer or
>> maintainer" needs to be emailed with your patch so that sufficiently
>> talented idiots such as myself, know who to mail.
>>
>> Although thinking about it we would be introducing yet another term
>> "supporting-maintainer" to which people would say "what is that"
>
> ...agree with this.
>
>> Feels a little less confusing to me to leave supporter as-is and just
>> document expectations for patch submission better.
>
> Hmm, how about this:
>
> scripts/get_maintainer.pl
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> Lee Jones <[email protected]> (maintainer[supported]:MULTIFUNCTION DEVICES
> (MFD))
> Andy Gross <[email protected]> (maintainer[volunteer]:ARM/QUALCOMM SUPPORT)
> Bjorn Andersson <[email protected]>
> (maintainer[volunteer]:ARM/QUALCOMM SUPPORT)
> Konrad Dybcio <[email protected]> (reviewer:ARM/QUALCOMM SUPPORT)
>
> Not totally sure about this myself. And there is a risk that any such
> change might break scripts that rely on the current approach used by
> scripts/get_maintainer.pl :-/

So it feels to me like the right thing to do is change get_maintainer
and accompanying documentation but, I'll wait to hear back from Jonathan.

---
bod