diff mbox

[PULL,31/32] MAINTAINERS: add all-match entry for qemu-devel@

Message ID 1455020031-8268-6-git-send-email-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paolo Bonzini Feb. 9, 2016, 12:13 p.m. UTC
From: Stephen Warren <swarren@wwwdotorg.org>

Add an entry to MAINTAINERS that matches every patch, and requests the
user send patches to qemu-devel@nongnu.org.

It's not 100% obvious to project newcomers that all patches should be sent
there; checkpatch doesn't say so, and since it mentions other lists to CC,
the wording "the list" from the SubmitAPatch wiki page can be taken
to mean only those lists, not the main list too.

The F: entries were taken from a similar entry in the Linux kernel.

Modify get_maintainer.pl so that the all-matching list entry doesn't
prevent the git fallback from ever triggering. The fallback now relies
on finding a real person in MAINTAINERS, not just a mailing list for
the relevant sub-community. This portion of the patch was suggested by
pbonzini@redhat.com.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Message-Id: <1454987065-12961-1-git-send-email-swarren@wwwdotorg.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS               | 5 +++++
 scripts/get_maintainer.pl | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Markus Armbruster Feb. 9, 2016, 1:47 p.m. UTC | #1
Not so fast :)

Paolo Bonzini <pbonzini@redhat.com> writes:

> From: Stephen Warren <swarren@wwwdotorg.org>
>
> Add an entry to MAINTAINERS that matches every patch, and requests the
> user send patches to qemu-devel@nongnu.org.
>
> It's not 100% obvious to project newcomers that all patches should be sent
> there; checkpatch doesn't say so, and since it mentions other lists to CC,

s/checkpatch/get_maintainer.pl/

> the wording "the list" from the SubmitAPatch wiki page can be taken
> to mean only those lists, not the main list too.
>
> The F: entries were taken from a similar entry in the Linux kernel.
>
> Modify get_maintainer.pl so that the all-matching list entry doesn't
> prevent the git fallback from ever triggering.

This suggests only the all-matching entry is exempt, but...

>                                                The fallback now relies
> on finding a real person in MAINTAINERS, not just a mailing list for

... in fact all lists now are.  Entries with L: and without M:

    Maintained: LINUX, POSIX, i386 target
    Odd fixes: GDB stub
    Orphan: Gumstix, Mipssim, Stable 0.10, Stable 0.14, Stable 0.15

For these, we now fall back to git.  Intentional?

If yes, mention it in the commit message.

If no, the easiest fix is to give them an M, if we can find a willing
victim.

Aside: we also have entries with neither L: nor M:

    Orphan: LSI53C895A, M68K, an5206, dummy_m68k, mcf5208

Odd.

> the relevant sub-community. This portion of the patch was suggested by
> pbonzini@redhat.com.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> Message-Id: <1454987065-12961-1-git-send-email-swarren@wwwdotorg.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  MAINTAINERS               | 5 +++++
>  scripts/get_maintainer.pl | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b6ed87a..2d78eea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -52,6 +52,11 @@ General Project Administration
>  ------------------------------
>  M: Peter Maydell <peter.maydell@linaro.org>
>  
> +All patches CC here

"All patches must be sent to this mailing list"

> +L: qemu-devel@nongnu.org
> +F: *
> +F: */
> +
>  Responsible Disclosure, Reporting Security Issues
>  ------------------------------
>  W: http://wiki.qemu.org/SecurityProcess
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 7dacf32..8261bcb 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -636,7 +636,7 @@ sub get_maintainers {
>  
>      if ($email) {
>  	if (! $interactive) {
> -	    $email_git_fallback = 0 if @email_to > 0 || @list_to > 0 || $email_git || $email_git_blame;
> +	    $email_git_fallback = 0 if @email_to > 0 || $email_git || $email_git_blame;
>  	    if ($email_git_fallback) {
>  	        print STDERR "get_maintainer.pl: No maintainers found, printing recent contributors.\n";
>  	        print STDERR "get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.\n";
Paolo Bonzini Feb. 9, 2016, 3:01 p.m. UTC | #2
On 09/02/2016 14:47, Markus Armbruster wrote:
>> >                                                The fallback now relies
>> > on finding a real person in MAINTAINERS, not just a mailing list for
> ... in fact all lists now are.  Entries with L: and without M:
> 
>     Maintained: LINUX, POSIX, i386 target
>     Odd fixes: GDB stub
>     Orphan: Gumstix, Mipssim, Stable 0.10, Stable 0.14, Stable 0.15

Not Stable 0.15 actually.

> For these, we now fall back to git.  Intentional?

Yes, intentional.  They are other instances of qemu-devel@ or, for
Gumstix, qemu-arm@.  For some of them it may make sense to give them an
M too, for stable versions we probably should drop them.

> If yes, mention it in the commit message.

It is mentioned.  For gumstix, it says sub-community right after where
you cut it. :) "The fallback now relies on finding a real person in
MAINTAINERS, not just a mailing list for the relevant sub-community".

For others, there's no reason why it should be different from the
catch-all entry.

> If no, the easiest fix is to give them an M, if we can find a willing
> victim.
> 
> Aside: we also have entries with neither L: nor M:
> 
>     Orphan: LSI53C895A, M68K, an5206, dummy_m68k, mcf5208
> 
> Odd.

Not too odd considering they're Orphan. :)  In fact the above entries
could have their L removed too (apart from Gumstix again).

Paolo
Markus Armbruster Feb. 9, 2016, 3:24 p.m. UTC | #3
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 09/02/2016 14:47, Markus Armbruster wrote:
>>> >                                                The fallback now relies
>>> > on finding a real person in MAINTAINERS, not just a mailing list for
>> ... in fact all lists now are.  Entries with L: and without M:
>> 
>>     Maintained: LINUX, POSIX, i386 target
>>     Odd fixes: GDB stub
>>     Orphan: Gumstix, Mipssim, Stable 0.10, Stable 0.14, Stable 0.15
>
> Not Stable 0.15 actually.

Oops, it's Stable 1.0.

>> For these, we now fall back to git.  Intentional?
>
> Yes, intentional.  They are other instances of qemu-devel@ or, for
> Gumstix, qemu-arm@.  For some of them it may make sense to give them an
> M too, for stable versions we probably should drop them.
>
>> If yes, mention it in the commit message.
>
> It is mentioned.  For gumstix, it says sub-community right after where
> you cut it. :) "The fallback now relies on finding a real person in
> MAINTAINERS, not just a mailing list for the relevant sub-community".

Yes, that sentence technically covers it, but I find the paragraph
confusing all the same.  Two ways to get my R-by:

1. Split the patch in two: part 1 is the get_maintainer.pl modification,
   part 2 is the MAINTAINERS update.  Each part does just one thing
   then, which allows simple commit messages.  That's what I'd do.

2. Keep the patch as is, but improve the commit message.  Here's my try:

    MAINTAINERS: Fall back to git unless we have a human, add qemu-devel@

    From: Stephen Warren <swarren@wwwdotorg.org>

    Add an entry to MAINTAINERS that matches every patch, and requests the
    user send patches to qemu-devel@nongnu.org.

    It's not 100% obvious to project newcomers that all patches should
    be sent there; get_maintainer.pl doesn't say so, and since it
    mentions other lists to CC, the wording "the list" from the
    SubmitAPatch wiki page can be taken to mean only those lists, not
    the main list too.

    The F: entries were taken from a similar entry in the Linux kernel.

    On its own, this would break fallback to git, because now every file
    has a maintainer of sorts.  Modify get_maintainer.pl so that mailing
    lists (L: lines) no longer prevent the fallback, only humans (M:
    entries).  This portion of the patch was suggested by
    pbonzini@redhat.com.

    Several pre-existing entries have a list but no human.  These now
    fall back to git.  That's a feature.

    Cc: Paolo Bonzini <pbonzini@redhat.com>
    Cc: Markus Armbruster <armbru@redhat.com>
    Cc: John Snow <jsnow@redhat.com>
    Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
    Message-Id: <1454987065-12961-1-git-send-email-swarren@wwwdotorg.org>
    [Commit message tweaked]
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
    Reviewed-by: Markus Armbruster <armbru@redhat.com>

In addition, consider changing the "All patches CC here" line to "All
patches must be sent to this mailing list".

> For others, there's no reason why it should be different from the
> catch-all entry.
>
>> If no, the easiest fix is to give them an M, if we can find a willing
>> victim.
>> 
>> Aside: we also have entries with neither L: nor M:
>> 
>>     Orphan: LSI53C895A, M68K, an5206, dummy_m68k, mcf5208
>> 
>> Odd.
>
> Not too odd considering they're Orphan. :)  In fact the above entries
> could have their L removed too (apart from Gumstix again).
>
> Paolo
Peter Maydell Feb. 9, 2016, 3:27 p.m. UTC | #4
On 9 February 2016 at 15:24, Markus Armbruster <armbru@redhat.com> wrote:
> 1. Split the patch in two: part 1 is the get_maintainer.pl modification,
>    part 2 is the MAINTAINERS update.  Each part does just one thing
>    then, which allows simple commit messages.  That's what I'd do.

I would favour this as well...

thanks
-- PMM
diff mbox

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index b6ed87a..2d78eea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -52,6 +52,11 @@  General Project Administration
 ------------------------------
 M: Peter Maydell <peter.maydell@linaro.org>
 
+All patches CC here
+L: qemu-devel@nongnu.org
+F: *
+F: */
+
 Responsible Disclosure, Reporting Security Issues
 ------------------------------
 W: http://wiki.qemu.org/SecurityProcess
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 7dacf32..8261bcb 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -636,7 +636,7 @@  sub get_maintainers {
 
     if ($email) {
 	if (! $interactive) {
-	    $email_git_fallback = 0 if @email_to > 0 || @list_to > 0 || $email_git || $email_git_blame;
+	    $email_git_fallback = 0 if @email_to > 0 || $email_git || $email_git_blame;
 	    if ($email_git_fallback) {
 	        print STDERR "get_maintainer.pl: No maintainers found, printing recent contributors.\n";
 	        print STDERR "get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.\n";