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

[Don Slutz <dslutz@verizon.com>]

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" <mst@redhat.com> writes:
>>
>>> On Wed, Oct 22, 2014 at 09:01:24AM +0200, Markus Armbruster wrote:
>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>>
>>>>> On Tue, Oct 21, 2014 at 03:34:46PM +0200, Markus Armbruster wrote:
>>>>>> Paolo Bonzini <pbonzini@redhat.com> 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 <lcapitulino@redhat.com> (commit_signer:1/2=50%)
>>>>      Eric Blake <eblake@redhat.com> (commit_signer:1/2=50%)
>>>>      Alexey Kardashevskiy <aik@ozlabs.ru> (commit_signer:1/2=50%)
>>>>      Laszlo Ersek <lersek@redhat.com> (commit_signer:1/2=50%)
>>>>      Amit Shah <amit.shah@redhat.com> (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: qemu-devel@nongnu.org
> +S: Odd Fixes
> +F: util/
>
>> Don proposed adding a catchall maintainer, and Peter refined
>> it to qemu-unmaintained@...  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 <dslutz@verizon.com>
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 <dslutz@verizon.com>
---
  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 <cnanakos@grnet.gr>
  M: Chrysostomos Nanakos <chris@include.gr>
  S: Maintained
  F: block/archipelago.c
+
+Everything not covered above
+----------------------------
+THE REST
+M: qemu-unmaintained@nongnu.org
+L: qemu-devel@nongnu.org
+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