| Message ID | 1455020031-8268-6-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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";
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
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
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 --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";