Re: [Qemu-devel] [PATCH] get_maintainer.pl: Default to --no-git-fallback

[Don Slutz]

On 10/22/14 04:29, Michael S. Tsirkin wrote:
> On Wed, Oct 22, 2014 at 10:03:53AM +0200, Markus Armbruster wrote:
> > "Michael S. Tsirkin" <address@hidden> writes:
> >
> > > On Wed, Oct 22, 2014 at 09:01:24AM +0200, Markus Armbruster wrote:
> > > > "Michael S. Tsirkin" <address@hidden> writes:
> > > >
> > > > > On Tue, Oct 21, 2014 at 03:34:46PM +0200, Markus Armbruster wrote:
> > > > > > Paolo Bonzini <address@hidden> writes:
> > > > > >
> > > > > > > On 10/20/2014 04:15 PM, Michael S. Tsirkin wrote:
> > > > > > > > What do you want to happen in this case?
> > > > > > > > Won't this cause even more patches to fall to the floor?
> > > > > > > >
> > > > > > > > The benefit seems marginal, the risk high.
> > > > > > > I agree with Michael.
> > > > > > >
> > > > > > > Can we detect if get_maintainer.pl is invoked as a cccmd, and in this
> > > > > > > case default to --no-git-fallback?  If it is invoked manually, I would
> > > > > > > like to show the committers (I will then cherry pick the right ones).
> > > > > > I don't like context-sensitive defaults.  Too much magic.
> > > > > >
> > > > > > What about this: if get_maintainer.pl comes up empty, it points you to
> > > > > > --git-fallback.
> > > > > This is exactly what it's doing now :)
> > > > Nope.  This is what it's doing now:
> > > >
> > > >      $ scripts/get_maintainer.pl -f util/cutils.c
> > > >      Luiz Capitulino <address@hidden> (commit_signer:1/2=50%)
> > > >      Eric Blake <address@hidden> (commit_signer:1/2=50%)
> > > >      Alexey Kardashevskiy <address@hidden> (commit_signer:1/2=50%)
> > > >      Laszlo Ersek <address@hidden> (commit_signer:1/2=50%)
> > > >      Amit Shah <address@hidden> (commit_signer:1/2=50%)
> > > >
> > > > A sufficiently seasoned contributor will spot the "commit_signer" tags,
> > > > and the output as a hint to find people to copy.  In this particular
> > > > case, he'll recognize the hint is useless.  Maybe he'll try something
> > > > like --git-since 2010 or --git-blame then.  I'd just peruse git-log.
> > > >
> > > > A less seasoned contributor will blindly copy all five.
> > > I give up. What's the correct answer?
> > > I frankly don't know whom should one copy on this file.
> > > Fabrice?
> > Fabrice would be a textbook example of a useless cc.
> >
> > I'd pick based on the patch's contents.  For instance, if it fixes a
> > function that is used by block stuff only, try copying block
> > maintainers.  You get the idea.  It's an art, not something a dumb
> > script can do.
> >
> > An inexperienced contributor probably won't be able to find out whom to
> > copy here.  Making him send out five mostly useless copies is not a
> > solution.
> Maybe disable fallback just for utils:
>
> +UTIL
> +M: address@hidden
> +S: Odd Fixes
> +F: util/
>
> > Don proposed adding a catchall maintainer, and Peter refined
> > it to address@hidden  This could serve as a formal cry "I have no
> > idea who could review this, please help me".
> >
> > [...]
> The list is a good idea. But I'd like a flag to avoid adding that
> automatically. Call it --expert or whatever. So need to write some
> code in get_maintainer.

I.E just what I proposed.  Here is an adjusted version.  I have not 
posted it
as a patch because the list is missing...

   -Don Slutz

From cf608abd785cc55c292215f2b7f18370eaf0969c Mon Sep 17 00:00:00 2001
From: Don Slutz <address@hidden>
Date: Mon, 20 Oct 2014 16:06:35 -0400
Subject: [PATCH] get_maintainer.pl: Add 'THE REST'

For an example of how this might look:

k=50;i=0;for sha in $(git log --no-merges --format=format:"%h" -$k);do
 let i=$i+1
 git show $sha >/tmp/a
 clear
 echo $i
 head -20 /tmp/a
 echo
 echo
 echo to:
 ./scripts/get_maintainer.pl --no-git-fallback --no-m /tmp/a
 echo
 echo cc:
 ./scripts/get_maintainer.pl --no-git-fallback --no-l /tmp/a
 echo
 echo to \(git-fallback\):
 ./scripts/get_maintainer.pl --git-fallback --no-m /tmp/a
 echo
 echo cc \(git-fallback\):
 ./scripts/get_maintainer.pl --git-fallback --no-l /tmp/a
 read foo
done;echo Check $i

Signed-off-by: Don Slutz <address@hidden>
---
 MAINTAINERS               |  9 +++++++++
 scripts/get_maintainer.pl | 15 +++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 206bf7e..14abb0b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1018,3 +1018,12 @@ M: Chrysostomos Nanakos <address@hidden>
 M: Chrysostomos Nanakos <address@hidden>
 S: Maintained
 F: block/archipelago.c
+
+Everything not covered above
+----------------------------
+THE REST
+M: address@hidden
+L: address@hidden
+S: Orphan
+F: *
+F: */
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index d5eee8c..b9580b0 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -37,6 +37,7 @@ my $email_hg_since = "-365";
 my $interactive = 0;
 my $email_remove_duplicates = 1;
 my $email_use_mailmap = 1;
+my $email_drop_the_rest_orphan_if_supporter_found = 1;
 my $output_multiline = 1;
 my $output_separator = ", ";
 my $output_roles = 0;
@@ -196,6 +197,7 @@ if (!GetOptions(
                'i|interactive!' => \$interactive,
                'remove-duplicates!' => \$email_remove_duplicates,
                'mailmap!' => \$email_use_mailmap,
+               'drop_the_rest_orphan!' => 
\$email_drop_the_rest_orphan_if_supporter_found,
                'm!' => \$email_maintainer,
                'n!' => \$email_usename,
                'l!' => \$email_list,
@@ -647,6 +649,19 @@ sub get_maintainers {
        }
     }

+    if ($email_drop_the_rest_orphan_if_supporter_found && $#email_to > 0) {
+        my @email_new;
+        my $do_replace = 0;
+        foreach my $email (@email_to) {
+            if ($email->[1] ne 'orphan minder:THE REST') {
+                $do_replace = 1;
+                push @email_new, $email;
+            }
+        }
+        @email_to = @email_new
+            if $do_replace;
+    }
+
     foreach my $email (@email_to, @list_to) {
        $email->[0] = deduplicate_email($email->[0]);
     }
--
1.8.4