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

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

[Markus Armbruster]

Contributors rely on this script to find maintainers to copy.  The
script falls back to git when no exact MAINTAINERS pattern matches.
When that happens, recent contributors get copied, which tends not be
particularly useful.  Some contributors find it even annoying.

Flip the default to "don't fall back to git".  Use --git-fallback to
ask it to fall back to git.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/get_maintainer.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 38334de..ec2d16f 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -28,7 +28,7 @@ my $email_git = 0;
 my $email_git_all_signature_types = 0;
 my $email_git_blame = 0;
 my $email_git_blame_signatures = 1;
-my $email_git_fallback = 1;
+my $email_git_fallback = 0;
 my $email_git_min_signatures = 1;
 my $email_git_max_maintainers = 5;
 my $email_git_min_percent = 5;
-- 
1.9.3

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

[Don Slutz]

I am happy with this so:

Reviewed-by: Don Slutz <dslutz@verizon.com>

    -Don Slutz

On 10/20/14 05:19, Markus Armbruster wrote:
> Contributors rely on this script to find maintainers to copy.  The
> script falls back to git when no exact MAINTAINERS pattern matches.
> When that happens, recent contributors get copied, which tends not be
> particularly useful.  Some contributors find it even annoying.
>
> Flip the default to "don't fall back to git".  Use --git-fallback to
> ask it to fall back to git.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   scripts/get_maintainer.pl | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 38334de..ec2d16f 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -28,7 +28,7 @@ my $email_git = 0;
>   my $email_git_all_signature_types = 0;
>   my $email_git_blame = 0;
>   my $email_git_blame_signatures = 1;
> -my $email_git_fallback = 1;
> +my $email_git_fallback = 0;
>   my $email_git_min_signatures = 1;
>   my $email_git_max_maintainers = 5;
>   my $email_git_min_percent = 5;

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

[Peter Maydell]

On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> Contributors rely on this script to find maintainers to copy.  The
> script falls back to git when no exact MAINTAINERS pattern matches.
> When that happens, recent contributors get copied, which tends not be
> particularly useful.  Some contributors find it even annoying.
>
> Flip the default to "don't fall back to git".  Use --git-fallback to
> ask it to fall back to git.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Good idea.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

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

[Michael S. Tsirkin]

On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> > Contributors rely on this script to find maintainers to copy.  The
> > script falls back to git when no exact MAINTAINERS pattern matches.
> > When that happens, recent contributors get copied, which tends not be
> > particularly useful.  Some contributors find it even annoying.
> >
> > Flip the default to "don't fall back to git".  Use --git-fallback to
> > ask it to fall back to git.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> Good idea.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> -- PMM

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 would be OK with this if you also go over history
and assign maintainers to all core files which lack
maintainers listed in MAINTAINERS.

I'm yet to see contributors who are annoyed but we
can always blacklist specific people.

-- 
MST

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

[Peter Maydell]

On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
>> > Contributors rely on this script to find maintainers to copy.  The
>> > script falls back to git when no exact MAINTAINERS pattern matches.
>> > When that happens, recent contributors get copied, which tends not be
>> > particularly useful.  Some contributors find it even annoying.
>> >
>> > Flip the default to "don't fall back to git".  Use --git-fallback to
>> > ask it to fall back to git.

>> Good idea.

> What do you want to happen in this case?

It should mail the people who are actually maintainers,
not anybody who happened to touch the code in the last
year.

> I'm yet to see contributors who are annoyed but we
> can always blacklist specific people.

At the moment I just don't use get_maintainers.pl at
all because I tried it a few times and it just cc'd
a bunch of irrelevant people...

I suspect anybody using it at the moment is either
using the --no-git-fallback flag or trimming the
cc list a lot.

thanks
-- PMM

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On 10/20/2014 03:19 AM, Markus Armbruster wrote:
> Contributors rely on this script to find maintainers to copy.  The
> script falls back to git when no exact MAINTAINERS pattern matches.
> When that happens, recent contributors get copied, which tends not be
> particularly useful.  Some contributors find it even annoying.

Definitely.

> 
> Flip the default to "don't fall back to git".  Use --git-fallback to
> ask it to fall back to git.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/get_maintainer.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

[Paolo Bonzini]

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).

Paolo

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

[Michael S. Tsirkin]

On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> >> > Contributors rely on this script to find maintainers to copy.  The
> >> > script falls back to git when no exact MAINTAINERS pattern matches.
> >> > When that happens, recent contributors get copied, which tends not be
> >> > particularly useful.  Some contributors find it even annoying.
> >> >
> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
> >> > ask it to fall back to git.
> 
> >> Good idea.
> 
> > What do you want to happen in this case?
> 
> It should mail the people who are actually maintainers,
> not anybody who happened to touch the code in the last
> year.

Right but as often as not there's no data about that
in MAINTAINERS.


> > I'm yet to see contributors who are annoyed but we
> > can always blacklist specific people.
> 
> At the moment I just don't use get_maintainers.pl at
> all because I tried it a few times and it just cc'd
> a bunch of irrelevant people...
> 
> I suspect anybody using it at the moment is either
> using the --no-git-fallback flag or trimming the
> cc list a lot.
> 
> thanks
> -- PMM

I'm using it: sometimes with --no-git-fallback, sometimes without.

IIUC the default is to have up to 5 people on the Cc list
(--git-max-maintainers).
It's not like it adds 200 random people, is it?

Anyway experienced contributors can figure it out IMHO.


Question in my mind is what do we want a casual contributor
to do if there's no one listed in MAINTAINERS.
"Look in MAINTAINERS, if not there, look in git log"
sounds very reasonable to me, better than "CC no one".

-- 
MST

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

[Don Slutz]

[-- Attachment #1: Type: text/plain, Size: 4315 bytes --]

On 10/20/14 15:03, Michael S. Tsirkin wrote:
> On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
>> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>>>> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
>>>>> Contributors rely on this script to find maintainers to copy.  The
>>>>> script falls back to git when no exact MAINTAINERS pattern matches.
>>>>> When that happens, recent contributors get copied, which tends not be
>>>>> particularly useful.  Some contributors find it even annoying.
>>>>>
>>>>> Flip the default to "don't fall back to git".  Use --git-fallback to
>>>>> ask it to fall back to git.
>>>> Good idea.
>>> What do you want to happen in this case?
>> It should mail the people who are actually maintainers,
>> not anybody who happened to touch the code in the last
>> year.
> Right but as often as not there's no data about that
> in MAINTAINERS.
>
>
>>> I'm yet to see contributors who are annoyed but we
>>> can always blacklist specific people.
>> At the moment I just don't use get_maintainers.pl at
>> all because I tried it a few times and it just cc'd
>> a bunch of irrelevant people...
>>
>> I suspect anybody using it at the moment is either
>> using the --no-git-fallback flag or trimming the
>> cc list a lot.
>>
>> thanks
>> -- PMM
> I'm using it: sometimes with --no-git-fallback, sometimes without.
>
> IIUC the default is to have up to 5 people on the Cc list
> (--git-max-maintainers).
> It's not like it adds 200 random people, is it?
>
> Anyway experienced contributors can figure it out IMHO.
>
>
> Question in my mind is what do we want a casual contributor
> to do if there's no one listed in MAINTAINERS.
> "Look in MAINTAINERS, if not there, look in git log"
> sounds very reasonable to me, better than "CC no one".
>

Here is a possible patch (based on a xen change).  It adds the special
supporter:THE REST

Which is listed at the end of MAINTAINERS.  I included a quick guess...

    -Don Slutz

 From 82bd0d1dcfc5d17d1b72e12b2da3c9bb51e09a48 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'

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
  MAINTAINERS               |  8 ++++++++
  scripts/get_maintainer.pl | 15 +++++++++++++++
  2 files changed, 23 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 206bf7e..527227a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1018,3 +1018,11 @@ M: Chrysostomos Nanakos <cnanakos@grnet.gr>
  M: Chrysostomos Nanakos <chris@include.gr>
  S: Maintained
  F: block/archipelago.c
+
+THE REST
+M: Michael S. Tsirkin <mst@redhat.com>
+M: Peter Maydell <peter.maydell@linaro.org>
+L: qemu-devel@nongnu.org
+S: Supported
+F: *
+F: */
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 38334de..eef16ee 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_supporter_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_supporter!' => 
\$email_drop_the_rest_supporter_if_supporter_found,
                 'm!' => \$email_maintainer,
                 'n!' => \$email_usename,
                 'l!' => \$email_list,
@@ -647,6 +649,19 @@ sub get_maintainers {
         }
      }

+    if ($email_drop_the_rest_supporter_if_supporter_found && $#email_to 
 > 0) {
+        my @email_new;
+        my $do_replace = 0;
+        foreach my $email (@email_to) {
+            if ($email->[1] ne 'supporter: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





[-- Attachment #2: 0001-get_maintainer.pl-Add-THE-REST.patch --]
[-- Type: text/x-patch, Size: 2117 bytes --]

>From 82bd0d1dcfc5d17d1b72e12b2da3c9bb51e09a48 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'

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 MAINTAINERS               |  8 ++++++++
 scripts/get_maintainer.pl | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 206bf7e..527227a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1018,3 +1018,11 @@ M: Chrysostomos Nanakos <cnanakos@grnet.gr>
 M: Chrysostomos Nanakos <chris@include.gr>
 S: Maintained
 F: block/archipelago.c
+
+THE REST
+M: Michael S. Tsirkin <mst@redhat.com>
+M: Peter Maydell <peter.maydell@linaro.org>
+L: qemu-devel@nongnu.org
+S: Supported
+F: *
+F: */
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 38334de..eef16ee 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_supporter_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_supporter!' => \$email_drop_the_rest_supporter_if_supporter_found,
 		'm!' => \$email_maintainer,
 		'n!' => \$email_usename,
 		'l!' => \$email_list,
@@ -647,6 +649,19 @@ sub get_maintainers {
 	}
     }
 
+    if ($email_drop_the_rest_supporter_if_supporter_found && $#email_to > 0) {
+        my @email_new;
+        my $do_replace = 0;
+        foreach my $email (@email_to) {
+            if ($email->[1] ne 'supporter: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

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

[Peter Maydell]

On 20 October 2014 21:10, Don Slutz <dslutz@verizon.com> wrote:
> Here is a possible patch (based on a xen change).  It adds the special
> supporter:THE REST
>
> Which is listed at the end of MAINTAINERS.  I included a quick guess...

> +THE REST
> +M: Michael S. Tsirkin <mst@redhat.com>
> +M: Peter Maydell <peter.maydell@linaro.org>
> +L: qemu-devel@nongnu.org
> +S: Supported
> +F: *
> +F: */

I don't know about MST but I think this is a terrible idea :-)
If I get cc'd on everything then the cc just becomes noise
(TBH I already don't put a huge weighting on qemu-devel mail
cc'd to me or not).

On the other hand, if we made this fallback cc
qemu-unmaintained@ or some similar list that might actually
be useful, since (a) it flags up for submitters that their
contribution may be in an unmaintained gap (b) anybody who
cares can easily go through the list and fish things out
(c) we might be prompted to fix missing MAINTAINERS entries.

-- PMM

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

[Thomas Huth]

On Mon, 20 Oct 2014 17:15:48 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> > On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> > > Contributors rely on this script to find maintainers to copy.  The
> > > script falls back to git when no exact MAINTAINERS pattern matches.
> > > When that happens, recent contributors get copied, which tends not be
> > > particularly useful.  Some contributors find it even annoying.
> > >
> > > Flip the default to "don't fall back to git".  Use --git-fallback to
> > > ask it to fall back to git.
> > >
> > > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > 
> > Good idea.
> > 
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > 
> > -- PMM
> 
> 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.

Well, IMHO, at least the current behaviour if git-fallback seems to be a
little bit too easygoing: I already got mails just because I once
reviewed a patch in the past and thus got listed with the
"Reviewed-by:" tag. I would not expect that behaviour when I run a
"get_maintainers" script (it's not called "get_reviewers", is it?).

Maybe you could at least remove the "Reviewed-by:" and "Acked-by:" from
the --git-fallback option so that it only checks for the SOBs?

 Thomas

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
>> > Contributors rely on this script to find maintainers to copy.  The
>> > script falls back to git when no exact MAINTAINERS pattern matches.
>> > When that happens, recent contributors get copied, which tends not be
>> > particularly useful.  Some contributors find it even annoying.
>> >
>> > Flip the default to "don't fall back to git".  Use --git-fallback to
>> > ask it to fall back to git.
>> >
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> 
>> Good idea.
>> 
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> 
>> -- PMM
>
> 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 would be OK with this if you also go over history
> and assign maintainers to all core files which lack
> maintainers listed in MAINTAINERS.

Define "core files".

I don't think I (or anyone) should *assign* maintainers.  We've always
let people volunteer for the maintainer role.  Prodding them to
volunteer is fine, but shanghaiing them outright is a different matter.

We do have too may files lacking maintainers.  See

    Subject: MAINTAINERS leaves too many files uncovered
    Date: Mon, 20 Oct 2014 11:19:44 +0200
    Message-ID: <87mw8rumhb.fsf@blackfin.pond.sub.org>
    https://lists.nongnu.org/archive/html/qemu-devel/2014-10/msg01951.html

> I'm yet to see contributors who are annoyed but we
> can always blacklist specific people.

Quite a few have grumbled, both in this thread and elsewhere.  Usually,
for every one who grumbles, there are several quietly annoyed.

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
>> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
>> >> > Contributors rely on this script to find maintainers to copy.  The
>> >> > script falls back to git when no exact MAINTAINERS pattern matches.
>> >> > When that happens, recent contributors get copied, which tends not be
>> >> > particularly useful.  Some contributors find it even annoying.
>> >> >
>> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
>> >> > ask it to fall back to git.
>> 
>> >> Good idea.
>> 
>> > What do you want to happen in this case?
>> 
>> It should mail the people who are actually maintainers,
>> not anybody who happened to touch the code in the last
>> year.
>
> Right but as often as not there's no data about that
> in MAINTAINERS.

The way to fix that is finding maintainers, not scatter-shooting patches
to random contributors in the vague hope of hitting someone who cares.

>> > I'm yet to see contributors who are annoyed but we
>> > can always blacklist specific people.
>> 
>> At the moment I just don't use get_maintainers.pl at
>> all because I tried it a few times and it just cc'd
>> a bunch of irrelevant people...
>> 
>> I suspect anybody using it at the moment is either
>> using the --no-git-fallback flag or trimming the
>> cc list a lot.
>> 
>> thanks
>> -- PMM
>
> I'm using it: sometimes with --no-git-fallback, sometimes without.

I'm using it, but I absolutely want to know when it falls back to git,
because then I want to cheack and trim or ignore its output every single
time.

> IIUC the default is to have up to 5 people on the Cc list
> (--git-max-maintainers).
> It's not like it adds 200 random people, is it?
>
> Anyway experienced contributors can figure it out IMHO.

Experienced contributors can figure out --git-fallback, too.

What we see is contributors, especially less experienced ones, copying
whatever get_maintainers.pl spits out, because they have no idea what
get_maintainers.pl actually does.

> Question in my mind is what do we want a casual contributor
> to do if there's no one listed in MAINTAINERS.
> "Look in MAINTAINERS, if not there, look in git log"
> sounds very reasonable to me, better than "CC no one".

But that's not what we do!  We do "copy whatever get_maintainers.pl
coughs up", which boils down to "use MAINTAINERS, if not there, grab
some random victims from git-log".

Perhaps we'd get slightly better results if get_maintainers.pl told its
users clearly about the two kinds of output it may produce: maintainers
(must be copied on patches), and recent contributors (you're in trouble;
copying some of them may or may not help).

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

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
> >> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> >> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> >> >> > Contributors rely on this script to find maintainers to copy.  The
> >> >> > script falls back to git when no exact MAINTAINERS pattern matches.
> >> >> > When that happens, recent contributors get copied, which tends not be
> >> >> > particularly useful.  Some contributors find it even annoying.
> >> >> >
> >> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
> >> >> > ask it to fall back to git.
> >> 
> >> >> Good idea.
> >> 
> >> > What do you want to happen in this case?
> >> 
> >> It should mail the people who are actually maintainers,
> >> not anybody who happened to touch the code in the last
> >> year.
> >
> > Right but as often as not there's no data about that
> > in MAINTAINERS.
> 
> The way to fix that is finding maintainers, not scatter-shooting patches
> to random contributors in the vague hope of hitting someone who cares.
> 
> >> > I'm yet to see contributors who are annoyed but we
> >> > can always blacklist specific people.
> >> 
> >> At the moment I just don't use get_maintainers.pl at
> >> all because I tried it a few times and it just cc'd
> >> a bunch of irrelevant people...
> >> 
> >> I suspect anybody using it at the moment is either
> >> using the --no-git-fallback flag or trimming the
> >> cc list a lot.
> >> 
> >> thanks
> >> -- PMM
> >
> > I'm using it: sometimes with --no-git-fallback, sometimes without.
> 
> I'm using it, but I absolutely want to know when it falls back to git,
> because then I want to cheack and trim or ignore its output every single
> time.


Well it tells you the role. What else is necessary?


> > IIUC the default is to have up to 5 people on the Cc list
> > (--git-max-maintainers).
> > It's not like it adds 200 random people, is it?
> >
> > Anyway experienced contributors can figure it out IMHO.
> 
> Experienced contributors can figure out --git-fallback, too.

Exactly.

> What we see is contributors, especially less experienced ones, copying
> whatever get_maintainers.pl spits out, because they have no idea what
> get_maintainers.pl actually does.

Exactly. And this seems better than just sending to qemu ML
and not copying anyone.

> > Question in my mind is what do we want a casual contributor
> > to do if there's no one listed in MAINTAINERS.
> > "Look in MAINTAINERS, if not there, look in git log"
> > sounds very reasonable to me, better than "CC no one".
> 
> But that's not what we do!  We do "copy whatever get_maintainers.pl
> coughs up", which boils down to "use MAINTAINERS, if not there, grab
> some random victims from git-log".

Sorry, what's the difference?
"look in" versus "random victims"? what makes them random?
Maybe you just want to increase git-min-percent?

> Perhaps we'd get slightly better results if get_maintainers.pl told its
> users clearly about the two kinds of output it may produce: maintainers
> (must be copied on patches), and recent contributors (you're in trouble;
> copying some of them may or may not help).

That's what it does: it reports the role, and the percent.
What's missing?

-- 
MST

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

[Gerd Hoffmann]

On Mo, 2014-10-20 at 20:38 +0200, Paolo Bonzini wrote:
> 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).

How about making "get_maintainer.pl --git-fallback" actually do what it
says?  Right now git it *not* used as fallback, it goes to git log
unconditionally, even if there are hits in MAINTAINERS ...

Different behavior depending on whenever stdout is a tty or not should
be doable too.

cheers,
  Gerd

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

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 01:09:19PM +0200, Gerd Hoffmann wrote:
> On Mo, 2014-10-20 at 20:38 +0200, Paolo Bonzini wrote:
> > 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).
> 
> How about making "get_maintainer.pl --git-fallback" actually do what it
> says?  Right now git it *not* used as fallback, it goes to git log
> unconditionally, even if there are hits in MAINTAINERS ...

It does?

How do you reproduce this behaviour?

$ ./scripts/get_maintainer.pl -f vl.c
Anthony Liguori <aliguori@amazon.com> (supporter:Main loop)

Seems to stop if it sees a maintainer.

> Different behavior depending on whenever stdout is a tty or not should
> be doable too.
> 
> cheers,
>   Gerd
> 
> 

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

[Gerd Hoffmann]

  Hi,

> > How about making "get_maintainer.pl --git-fallback" actually do what it
> > says?  Right now git it *not* used as fallback, it goes to git log
> > unconditionally, even if there are hits in MAINTAINERS ...
> 
> It does?
> 
> How do you reproduce this behaviour?
> 
> $ ./scripts/get_maintainer.pl -f vl.c
> Anthony Liguori <aliguori@amazon.com> (supporter:Main loop)
> 
> Seems to stop if it sees a maintainer.

nilsson kraxel ~/projects/qemu (usb.work)# scripts/get_maintainer.pl -f
ui/vnc.c
Anthony Liguori <aliguori@amazon.com> (odd fixer:Graphics)
Gerd Hoffmann <kraxel@redhat.com> (odd
fixer:Graphics,commit_signer:21/24=88%)
Peter Lieven <pl@kamp.de> (commit_signer:12/24=50%)
Wenchao Xia <wenchaoqemu@gmail.com> (commit_signer:7/24=29%)
Markus Armbruster <armbru@redhat.com> (commit_signer:3/24=12%)
Luiz Capitulino <lcapitulino@redhat.com> (commit_signer:3/24=12%)

Maybe it depends on maintainer status ...

cheers,
  Gerd

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

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 01:23:49PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > How about making "get_maintainer.pl --git-fallback" actually do what it
> > > says?  Right now git it *not* used as fallback, it goes to git log
> > > unconditionally, even if there are hits in MAINTAINERS ...
> > 
> > It does?
> > 
> > How do you reproduce this behaviour?
> > 
> > $ ./scripts/get_maintainer.pl -f vl.c
> > Anthony Liguori <aliguori@amazon.com> (supporter:Main loop)
> > 
> > Seems to stop if it sees a maintainer.
> 
> nilsson kraxel ~/projects/qemu (usb.work)# scripts/get_maintainer.pl -f
> ui/vnc.c
> Anthony Liguori <aliguori@amazon.com> (odd fixer:Graphics)
> Gerd Hoffmann <kraxel@redhat.com> (odd
> fixer:Graphics,commit_signer:21/24=88%)
> Peter Lieven <pl@kamp.de> (commit_signer:12/24=50%)
> Wenchao Xia <wenchaoqemu@gmail.com> (commit_signer:7/24=29%)
> Markus Armbruster <armbru@redhat.com> (commit_signer:3/24=12%)
> Luiz Capitulino <lcapitulino@redhat.com> (commit_signer:3/24=12%)
> 
> Maybe it depends on maintainer status ...
> 
> cheers,
>   Gerd
> 

Yes and that's a bug.
Thanks for the report.
Patch send, maybe that will be enough.


-- 
MST

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
>> >> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> >> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
>> >> >> > Contributors rely on this script to find maintainers to copy.  The
>> >> >> > script falls back to git when no exact MAINTAINERS pattern matches.
>> >> >> > When that happens, recent contributors get copied, which tends not be
>> >> >> > particularly useful.  Some contributors find it even annoying.
>> >> >> >
>> >> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
>> >> >> > ask it to fall back to git.
>> >> 
>> >> >> Good idea.
>> >> 
>> >> > What do you want to happen in this case?
>> >> 
>> >> It should mail the people who are actually maintainers,
>> >> not anybody who happened to touch the code in the last
>> >> year.
>> >
>> > Right but as often as not there's no data about that
>> > in MAINTAINERS.
>> 
>> The way to fix that is finding maintainers, not scatter-shooting patches
>> to random contributors in the vague hope of hitting someone who cares.
>> 
>> >> > I'm yet to see contributors who are annoyed but we
>> >> > can always blacklist specific people.
>> >> 
>> >> At the moment I just don't use get_maintainers.pl at
>> >> all because I tried it a few times and it just cc'd
>> >> a bunch of irrelevant people...
>> >> 
>> >> I suspect anybody using it at the moment is either
>> >> using the --no-git-fallback flag or trimming the
>> >> cc list a lot.
>> >> 
>> >> thanks
>> >> -- PMM
>> >
>> > I'm using it: sometimes with --no-git-fallback, sometimes without.
>> 
>> I'm using it, but I absolutely want to know when it falls back to git,
>> because then I want to cheack and trim or ignore its output every single
>> time.
>
>
> Well it tells you the role. What else is necessary?

For my own use in sending patches, nothing.  I know how to use it to
help me copy the right people.

>> > IIUC the default is to have up to 5 people on the Cc list
>> > (--git-max-maintainers).
>> > It's not like it adds 200 random people, is it?
>> >
>> > Anyway experienced contributors can figure it out IMHO.
>> 
>> Experienced contributors can figure out --git-fallback, too.
>
> Exactly.
>
>> What we see is contributors, especially less experienced ones, copying
>> whatever get_maintainers.pl spits out, because they have no idea what
>> get_maintainers.pl actually does.
>
> Exactly. And this seems better than just sending to qemu ML
> and not copying anyone.

That's where we disagree.

Personally, I don't mind getting punished for contributing patches by
getting copied indiscriminately all that much.  It's a drain on my time,
but I can cope.  However, I know people who do mind, and some of them
have spoken up in this thread.

Copying people is not free.  You should *think* before you copy.

An entry in MAINTAINERS dispenses you from this obligation, because the
people listed explicitly asked for a copy.

Finding someone in git-log does not!

get_maintainers.pl encourages its users to treat people found in git-log
exactly like the ones in MAINTAINERS.  Treating them the same is
*wrong*.

>> > Question in my mind is what do we want a casual contributor
>> > to do if there's no one listed in MAINTAINERS.
>> > "Look in MAINTAINERS, if not there, look in git log"
>> > sounds very reasonable to me, better than "CC no one".
>> 
>> But that's not what we do!  We do "copy whatever get_maintainers.pl
>> coughs up", which boils down to "use MAINTAINERS, if not there, grab
>> some random victims from git-log".
>
> Sorry, what's the difference?
> "look in" versus "random victims"? what makes them random?

The difference is using get_maintainers.pl to help finding whom to copy
vs. blindly copying whoever get_maintainers.pl coughs up.

> Maybe you just want to increase git-min-percent?
>
>> Perhaps we'd get slightly better results if get_maintainers.pl told its
>> users clearly about the two kinds of output it may produce: maintainers
>> (must be copied on patches), and recent contributors (you're in trouble;
>> copying some of them may or may not help).
>
> That's what it does: it reports the role, and the percent.

Boldly assumes the user of get_maintainers.pl knows what it does, and
knows how to interpret runes like (commit_signer:14/22=64%).

> What's missing?

What's really missing is decent coverage by MAINTAINERS.  I figure my
patch is controversial only because MAINTAINERS is so woefully
incomplete.

My patch to get_maintainers.pl triggered a whole thread, while the
message I sent on MAINTAINERS coverage got just one reply so far, and
even that one's really just about get_maintainers.pl.  Disappointing.
Looks like we're still looking for an easy technical fix.  I doubt there
is one.

If you have better ideas on how to mitigate the excessive and useless
copying we now see, please post a patch.

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

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
> >> >> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> >> >> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> >> >> >> > Contributors rely on this script to find maintainers to copy.  The
> >> >> >> > script falls back to git when no exact MAINTAINERS pattern matches.
> >> >> >> > When that happens, recent contributors get copied, which tends not be
> >> >> >> > particularly useful.  Some contributors find it even annoying.
> >> >> >> >
> >> >> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
> >> >> >> > ask it to fall back to git.
> >> >> 
> >> >> >> Good idea.
> >> >> 
> >> >> > What do you want to happen in this case?
> >> >> 
> >> >> It should mail the people who are actually maintainers,
> >> >> not anybody who happened to touch the code in the last
> >> >> year.
> >> >
> >> > Right but as often as not there's no data about that
> >> > in MAINTAINERS.
> >> 
> >> The way to fix that is finding maintainers, not scatter-shooting patches
> >> to random contributors in the vague hope of hitting someone who cares.
> >> 
> >> >> > I'm yet to see contributors who are annoyed but we
> >> >> > can always blacklist specific people.
> >> >> 
> >> >> At the moment I just don't use get_maintainers.pl at
> >> >> all because I tried it a few times and it just cc'd
> >> >> a bunch of irrelevant people...
> >> >> 
> >> >> I suspect anybody using it at the moment is either
> >> >> using the --no-git-fallback flag or trimming the
> >> >> cc list a lot.
> >> >> 
> >> >> thanks
> >> >> -- PMM
> >> >
> >> > I'm using it: sometimes with --no-git-fallback, sometimes without.
> >> 
> >> I'm using it, but I absolutely want to know when it falls back to git,
> >> because then I want to cheack and trim or ignore its output every single
> >> time.
> >
> >
> > Well it tells you the role. What else is necessary?
> 
> For my own use in sending patches, nothing.  I know how to use it to
> help me copy the right people.
> 
> >> > IIUC the default is to have up to 5 people on the Cc list
> >> > (--git-max-maintainers).
> >> > It's not like it adds 200 random people, is it?
> >> >
> >> > Anyway experienced contributors can figure it out IMHO.
> >> 
> >> Experienced contributors can figure out --git-fallback, too.
> >
> > Exactly.
> >
> >> What we see is contributors, especially less experienced ones, copying
> >> whatever get_maintainers.pl spits out, because they have no idea what
> >> get_maintainers.pl actually does.
> >
> > Exactly. And this seems better than just sending to qemu ML
> > and not copying anyone.
> 
> That's where we disagree.
> 
> Personally, I don't mind getting punished for contributing patches by
> getting copied indiscriminately all that much.  It's a drain on my time,
> but I can cope.  However, I know people who do mind, and some of them
> have spoken up in this thread.
> 
> Copying people is not free.  You should *think* before you copy.
> 
> An entry in MAINTAINERS dispenses you from this obligation, because the
> people listed explicitly asked for a copy.
> 
> Finding someone in git-log does not!
> 
> get_maintainers.pl encourages its users to treat people found in git-log
> exactly like the ones in MAINTAINERS.  Treating them the same is
> *wrong*.
> 
> >> > Question in my mind is what do we want a casual contributor
> >> > to do if there's no one listed in MAINTAINERS.
> >> > "Look in MAINTAINERS, if not there, look in git log"
> >> > sounds very reasonable to me, better than "CC no one".
> >> 
> >> But that's not what we do!  We do "copy whatever get_maintainers.pl
> >> coughs up", which boils down to "use MAINTAINERS, if not there, grab
> >> some random victims from git-log".
> >
> > Sorry, what's the difference?
> > "look in" versus "random victims"? what makes them random?
> 
> The difference is using get_maintainers.pl to help finding whom to copy
> vs. blindly copying whoever get_maintainers.pl coughs up.
> 
> > Maybe you just want to increase git-min-percent?
> >
> >> Perhaps we'd get slightly better results if get_maintainers.pl told its
> >> users clearly about the two kinds of output it may produce: maintainers
> >> (must be copied on patches), and recent contributors (you're in trouble;
> >> copying some of them may or may not help).
> >
> > That's what it does: it reports the role, and the percent.
> 
> Boldly assumes the user of get_maintainers.pl knows what it does, and
> knows how to interpret runes like (commit_signer:14/22=64%).

OK so you would like a flag for a more readable output?
Sounds very reasonable.

> > What's missing?
> 
> What's really missing is decent coverage by MAINTAINERS.  I figure my
> patch is controversial only because MAINTAINERS is so woefully
> incomplete.

In fact if MAINTAINERS covered everything your patch won't be needed
right?

> My patch to get_maintainers.pl triggered a whole thread, while the
> message I sent on MAINTAINERS coverage got just one reply so far, and
> even that one's really just about get_maintainers.pl.  Disappointing.
> Looks like we're still looking for an easy technical fix.  I doubt there
> is one.

At least for myself, that's because I'm Cc'd directly on the patch
but not on the MAINTAINERS coverage mail.
And that's ... because get_maintainers picks my mail from git?

See how it's useful now?


> If you have better ideas on how to mitigate the excessive and useless
> copying we now see, please post a patch.

We need more maintainers :)

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
>> >> >> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> >> >> >> On 20 October 2014 10:19, Markus Armbruster
>> >> >> >> <armbru@redhat.com> wrote:
>> >> >> >> > Contributors rely on this script to find maintainers to copy.  The
>> >> >> >> > script falls back to git when no exact MAINTAINERS pattern matches.
>> >> >> >> > When that happens, recent contributors get copied, which
>> >> >> >> > tends not be
>> >> >> >> > particularly useful.  Some contributors find it even annoying.
>> >> >> >> >
>> >> >> >> > Flip the default to "don't fall back to git".  Use
>> >> >> >> > --git-fallback to
>> >> >> >> > ask it to fall back to git.
>> >> >> 
>> >> >> >> Good idea.
>> >> >> 
>> >> >> > What do you want to happen in this case?
>> >> >> 
>> >> >> It should mail the people who are actually maintainers,
>> >> >> not anybody who happened to touch the code in the last
>> >> >> year.
>> >> >
>> >> > Right but as often as not there's no data about that
>> >> > in MAINTAINERS.
>> >> 
>> >> The way to fix that is finding maintainers, not scatter-shooting patches
>> >> to random contributors in the vague hope of hitting someone who cares.
>> >> 
>> >> >> > I'm yet to see contributors who are annoyed but we
>> >> >> > can always blacklist specific people.
>> >> >> 
>> >> >> At the moment I just don't use get_maintainers.pl at
>> >> >> all because I tried it a few times and it just cc'd
>> >> >> a bunch of irrelevant people...
>> >> >> 
>> >> >> I suspect anybody using it at the moment is either
>> >> >> using the --no-git-fallback flag or trimming the
>> >> >> cc list a lot.
>> >> >> 
>> >> >> thanks
>> >> >> -- PMM
>> >> >
>> >> > I'm using it: sometimes with --no-git-fallback, sometimes without.
>> >> 
>> >> I'm using it, but I absolutely want to know when it falls back to git,
>> >> because then I want to cheack and trim or ignore its output every single
>> >> time.
>> >
>> >
>> > Well it tells you the role. What else is necessary?
>> 
>> For my own use in sending patches, nothing.  I know how to use it to
>> help me copy the right people.
>> 
>> >> > IIUC the default is to have up to 5 people on the Cc list
>> >> > (--git-max-maintainers).
>> >> > It's not like it adds 200 random people, is it?
>> >> >
>> >> > Anyway experienced contributors can figure it out IMHO.
>> >> 
>> >> Experienced contributors can figure out --git-fallback, too.
>> >
>> > Exactly.
>> >
>> >> What we see is contributors, especially less experienced ones, copying
>> >> whatever get_maintainers.pl spits out, because they have no idea what
>> >> get_maintainers.pl actually does.
>> >
>> > Exactly. And this seems better than just sending to qemu ML
>> > and not copying anyone.
>> 
>> That's where we disagree.
>> 
>> Personally, I don't mind getting punished for contributing patches by
>> getting copied indiscriminately all that much.  It's a drain on my time,
>> but I can cope.  However, I know people who do mind, and some of them
>> have spoken up in this thread.
>> 
>> Copying people is not free.  You should *think* before you copy.
>> 
>> An entry in MAINTAINERS dispenses you from this obligation, because the
>> people listed explicitly asked for a copy.
>> 
>> Finding someone in git-log does not!
>> 
>> get_maintainers.pl encourages its users to treat people found in git-log
>> exactly like the ones in MAINTAINERS.  Treating them the same is
>> *wrong*.
>> 
>> >> > Question in my mind is what do we want a casual contributor
>> >> > to do if there's no one listed in MAINTAINERS.
>> >> > "Look in MAINTAINERS, if not there, look in git log"
>> >> > sounds very reasonable to me, better than "CC no one".
>> >> 
>> >> But that's not what we do!  We do "copy whatever get_maintainers.pl
>> >> coughs up", which boils down to "use MAINTAINERS, if not there, grab
>> >> some random victims from git-log".
>> >
>> > Sorry, what's the difference?
>> > "look in" versus "random victims"? what makes them random?
>> 
>> The difference is using get_maintainers.pl to help finding whom to copy
>> vs. blindly copying whoever get_maintainers.pl coughs up.
>> 
>> > Maybe you just want to increase git-min-percent?
>> >
>> >> Perhaps we'd get slightly better results if get_maintainers.pl told its
>> >> users clearly about the two kinds of output it may produce: maintainers
>> >> (must be copied on patches), and recent contributors (you're in trouble;
>> >> copying some of them may or may not help).
>> >
>> > That's what it does: it reports the role, and the percent.
>> 
>> Boldly assumes the user of get_maintainers.pl knows what it does, and
>> knows how to interpret runes like (commit_signer:14/22=64%).
>
> OK so you would like a flag for a more readable output?
> Sounds very reasonable.

Inexperienced contributors are unlikely to find a flag, so it better be
the default.

>> > What's missing?
>> 
>> What's really missing is decent coverage by MAINTAINERS.  I figure my
>> patch is controversial only because MAINTAINERS is so woefully
>> incomplete.
>
> In fact if MAINTAINERS covered everything your patch won't be needed
> right?

Correct.  The more MAINTAINERS covers, the less of a difference my patch
makes.

>> My patch to get_maintainers.pl triggered a whole thread, while the
>> message I sent on MAINTAINERS coverage got just one reply so far, and
>> even that one's really just about get_maintainers.pl.  Disappointing.
>> Looks like we're still looking for an easy technical fix.  I doubt there
>> is one.
>
> At least for myself, that's because I'm Cc'd directly on the patch
> but not on the MAINTAINERS coverage mail.
> And that's ... because get_maintainers picks my mail from git?
>
> See how it's useful now?

Except that's not what happened.

    $ scripts/get_maintainer.pl --git-fallback -f scripts/get_maintainer.pl 

No output.  I picked you from git-log manually.

>> If you have better ideas on how to mitigate the excessive and useless
>> copying we now see, please post a patch.
>
> We need more maintainers :)

Yes, we do.  Until we got them, we need fewer useless copies.

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

[Markus Armbruster]

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.

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

[Paolo Bonzini]

On 10/21/2014 03:34 PM, 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.

Sounds fair enough!

Paolo

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

[Kirill Batuzov]

On Tue, 21 Oct 2014, Markus Armbruster wrote:

> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
> >> > Contributors rely on this script to find maintainers to copy.  The
> >> > script falls back to git when no exact MAINTAINERS pattern matches.
> >> > When that happens, recent contributors get copied, which tends not be
> >> > particularly useful.  Some contributors find it even annoying.
> >> >
> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
> >> > ask it to fall back to git.
> >> >
> >> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> 
> >> Good idea.
> >> 
> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >> 
> >> -- PMM
> >
> > 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 would be OK with this if you also go over history
> > and assign maintainers to all core files which lack
> > maintainers listed in MAINTAINERS.
> 
> Define "core files".
>

Files implementing common infrastructure used in different guests on
different hosts? This probably is the least covered in MAINTAINERS part.
MAINTAINERS covers pretty well target architectures, host architectures
and guest machines (each of them is a well-defined independent subsystem).
On the other hand a lot of common files are missing from MAINTAINERS:
cpu-exec.c, hw/core/*.c, cputlb.c etc.

> I don't think I (or anyone) should *assign* maintainers.  We've always
> let people volunteer for the maintainer role.  Prodding them to
> volunteer is fine, but shanghaiing them outright is a different matter.
> 

May be we can start searching for volunteers by making a list of
unmaintained files grouped by subsystems? It is hard to find volunteers
when we do not know exactly what we need them for.

> We do have too may files lacking maintainers.  See
> 
>     Subject: MAINTAINERS leaves too many files uncovered
>     Date: Mon, 20 Oct 2014 11:19:44 +0200
>     Message-ID: <87mw8rumhb.fsf@blackfin.pond.sub.org>
>     https://lists.nongnu.org/archive/html/qemu-devel/2014-10/msg01951.html
> 
> > I'm yet to see contributors who are annoyed but we
> > can always blacklist specific people.
> 
> Quite a few have grumbled, both in this thread and elsewhere.  Usually,
> for every one who grumbles, there are several quietly annoyed.
> 
> 

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

[Kirill Batuzov]

On Tue, 21 Oct 2014, 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.
>

I am in favor of this. Empty output does not tell a new contributors what
to do next. So either this or document it on corresponding wiki page.

-- 
Kirill

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

[Markus Armbruster]

Kirill Batuzov <batuzovk@ispras.ru> writes:

> On Tue, 21 Oct 2014, Markus Armbruster wrote:
>
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> >> On 20 October 2014 10:19, Markus Armbruster <armbru@redhat.com> wrote:
>> >> > Contributors rely on this script to find maintainers to copy.  The
>> >> > script falls back to git when no exact MAINTAINERS pattern matches.
>> >> > When that happens, recent contributors get copied, which tends not be
>> >> > particularly useful.  Some contributors find it even annoying.
>> >> >
>> >> > Flip the default to "don't fall back to git".  Use --git-fallback to
>> >> > ask it to fall back to git.
>> >> >
>> >> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> 
>> >> Good idea.
>> >> 
>> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> >> 
>> >> -- PMM
>> >
>> > 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 would be OK with this if you also go over history
>> > and assign maintainers to all core files which lack
>> > maintainers listed in MAINTAINERS.
>> 
>> Define "core files".
>>
>
> Files implementing common infrastructure used in different guests on
> different hosts? This probably is the least covered in MAINTAINERS part.
> MAINTAINERS covers pretty well target architectures, host architectures
> and guest machines (each of them is a well-defined independent subsystem).
> On the other hand a lot of common files are missing from MAINTAINERS:
> cpu-exec.c, hw/core/*.c, cputlb.c etc.
>
>> I don't think I (or anyone) should *assign* maintainers.  We've always
>> let people volunteer for the maintainer role.  Prodding them to
>> volunteer is fine, but shanghaiing them outright is a different matter.
>> 
>
> May be we can start searching for volunteers by making a list of
> unmaintained files grouped by subsystems? It is hard to find volunteers
> when we do not know exactly what we need them for.

Yes.  I listed directories with many files at the end of this message:

>> We do have too may files lacking maintainers.  See
>> 
>>     Subject: MAINTAINERS leaves too many files uncovered
>>     Date: Mon, 20 Oct 2014 11:19:44 +0200
>>     Message-ID: <87mw8rumhb.fsf@blackfin.pond.sub.org>
>>     https://lists.nongnu.org/archive/html/qemu-devel/2014-10/msg01951.html

The message also shows how to list unmaintained files:

    $ for i in `git-ls-files`; do [ "`scripts/get_maintainer.pl -f --no-git-fallback $i`" ] || echo $i; done

I didn't include the full list because it's fairly big (~65KiB).

[...]

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

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 03:29:14PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote:
> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> 
> >> >> > On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
> >> >> >> On 20 October 2014 15:15, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
> >> >> >> >> On 20 October 2014 10:19, Markus Armbruster
> >> >> >> >> <armbru@redhat.com> wrote:
> >> >> >> >> > Contributors rely on this script to find maintainers to copy.  The
> >> >> >> >> > script falls back to git when no exact MAINTAINERS pattern matches.
> >> >> >> >> > When that happens, recent contributors get copied, which
> >> >> >> >> > tends not be
> >> >> >> >> > particularly useful.  Some contributors find it even annoying.
> >> >> >> >> >
> >> >> >> >> > Flip the default to "don't fall back to git".  Use
> >> >> >> >> > --git-fallback to
> >> >> >> >> > ask it to fall back to git.
> >> >> >> 
> >> >> >> >> Good idea.
> >> >> >> 
> >> >> >> > What do you want to happen in this case?
> >> >> >> 
> >> >> >> It should mail the people who are actually maintainers,
> >> >> >> not anybody who happened to touch the code in the last
> >> >> >> year.
> >> >> >
> >> >> > Right but as often as not there's no data about that
> >> >> > in MAINTAINERS.
> >> >> 
> >> >> The way to fix that is finding maintainers, not scatter-shooting patches
> >> >> to random contributors in the vague hope of hitting someone who cares.
> >> >> 
> >> >> >> > I'm yet to see contributors who are annoyed but we
> >> >> >> > can always blacklist specific people.
> >> >> >> 
> >> >> >> At the moment I just don't use get_maintainers.pl at
> >> >> >> all because I tried it a few times and it just cc'd
> >> >> >> a bunch of irrelevant people...
> >> >> >> 
> >> >> >> I suspect anybody using it at the moment is either
> >> >> >> using the --no-git-fallback flag or trimming the
> >> >> >> cc list a lot.
> >> >> >> 
> >> >> >> thanks
> >> >> >> -- PMM
> >> >> >
> >> >> > I'm using it: sometimes with --no-git-fallback, sometimes without.
> >> >> 
> >> >> I'm using it, but I absolutely want to know when it falls back to git,
> >> >> because then I want to cheack and trim or ignore its output every single
> >> >> time.
> >> >
> >> >
> >> > Well it tells you the role. What else is necessary?
> >> 
> >> For my own use in sending patches, nothing.  I know how to use it to
> >> help me copy the right people.
> >> 
> >> >> > IIUC the default is to have up to 5 people on the Cc list
> >> >> > (--git-max-maintainers).
> >> >> > It's not like it adds 200 random people, is it?
> >> >> >
> >> >> > Anyway experienced contributors can figure it out IMHO.
> >> >> 
> >> >> Experienced contributors can figure out --git-fallback, too.
> >> >
> >> > Exactly.
> >> >
> >> >> What we see is contributors, especially less experienced ones, copying
> >> >> whatever get_maintainers.pl spits out, because they have no idea what
> >> >> get_maintainers.pl actually does.
> >> >
> >> > Exactly. And this seems better than just sending to qemu ML
> >> > and not copying anyone.
> >> 
> >> That's where we disagree.
> >> 
> >> Personally, I don't mind getting punished for contributing patches by
> >> getting copied indiscriminately all that much.  It's a drain on my time,
> >> but I can cope.  However, I know people who do mind, and some of them
> >> have spoken up in this thread.
> >> 
> >> Copying people is not free.  You should *think* before you copy.
> >> 
> >> An entry in MAINTAINERS dispenses you from this obligation, because the
> >> people listed explicitly asked for a copy.
> >> 
> >> Finding someone in git-log does not!
> >> 
> >> get_maintainers.pl encourages its users to treat people found in git-log
> >> exactly like the ones in MAINTAINERS.  Treating them the same is
> >> *wrong*.
> >> 
> >> >> > Question in my mind is what do we want a casual contributor
> >> >> > to do if there's no one listed in MAINTAINERS.
> >> >> > "Look in MAINTAINERS, if not there, look in git log"
> >> >> > sounds very reasonable to me, better than "CC no one".
> >> >> 
> >> >> But that's not what we do!  We do "copy whatever get_maintainers.pl
> >> >> coughs up", which boils down to "use MAINTAINERS, if not there, grab
> >> >> some random victims from git-log".
> >> >
> >> > Sorry, what's the difference?
> >> > "look in" versus "random victims"? what makes them random?
> >> 
> >> The difference is using get_maintainers.pl to help finding whom to copy
> >> vs. blindly copying whoever get_maintainers.pl coughs up.
> >> 
> >> > Maybe you just want to increase git-min-percent?
> >> >
> >> >> Perhaps we'd get slightly better results if get_maintainers.pl told its
> >> >> users clearly about the two kinds of output it may produce: maintainers
> >> >> (must be copied on patches), and recent contributors (you're in trouble;
> >> >> copying some of them may or may not help).
> >> >
> >> > That's what it does: it reports the role, and the percent.
> >> 
> >> Boldly assumes the user of get_maintainers.pl knows what it does, and
> >> knows how to interpret runes like (commit_signer:14/22=64%).
> >
> > OK so you would like a flag for a more readable output?
> > Sounds very reasonable.
> 
> Inexperienced contributors are unlikely to find a flag, so it better be
> the default.

Fine with me, send a patch. Might be useful for Linux where we got
this from, too.

> >> > What's missing?
> >> 
> >> What's really missing is decent coverage by MAINTAINERS.  I figure my
> >> patch is controversial only because MAINTAINERS is so woefully
> >> incomplete.
> >
> > In fact if MAINTAINERS covered everything your patch won't be needed
> > right?
> 
> Correct.  The more MAINTAINERS covers, the less of a difference my patch
> makes.
> 
> >> My patch to get_maintainers.pl triggered a whole thread, while the
> >> message I sent on MAINTAINERS coverage got just one reply so far, and
> >> even that one's really just about get_maintainers.pl.  Disappointing.
> >> Looks like we're still looking for an easy technical fix.  I doubt there
> >> is one.
> >
> > At least for myself, that's because I'm Cc'd directly on the patch
> > but not on the MAINTAINERS coverage mail.
> > And that's ... because get_maintainers picks my mail from git?
> >
> > See how it's useful now?
> 
> Except that's not what happened.
> 
>     $ scripts/get_maintainer.pl --git-fallback -f scripts/get_maintainer.pl 
> 
> No output.  I picked you from git-log manually.

Weird.
It works for me:
 ./scripts/get_maintainer.pl -f scripts/get_maintainer.pl 
"Michael S. Tsirkin" <mst@redhat.com> (commit_signer:1/1=100%)                                                                                    

Maybe --git-fallback is broken?
Another reason to defer this patch ...

> >> If you have better ideas on how to mitigate the excessive and useless
> >> copying we now see, please post a patch.
> >
> > We need more maintainers :)
> 
> Yes, we do.  Until we got them, we need fewer useless copies.

Classical long tail problem. It's hard to get rid of useless copies
without getting rid of useful ones :)

-- 
MST

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

[Michael S. Tsirkin]

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 :)

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

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 04:15:08PM +0200, Markus Armbruster wrote:
> >> We do have too may files lacking maintainers.  See
> >> 
> >>     Subject: MAINTAINERS leaves too many files uncovered
> >>     Date: Mon, 20 Oct 2014 11:19:44 +0200
> >>     Message-ID: <87mw8rumhb.fsf@blackfin.pond.sub.org>
> >>     https://lists.nongnu.org/archive/html/qemu-devel/2014-10/msg01951.html
> 
> The message also shows how to list unmaintained files:
> 
>     $ for i in `git-ls-files`; do [ "`scripts/get_maintainer.pl -f --no-git-fallback $i`" ] || echo $i; done
> 
> I didn't include the full list because it's fairly big (~65KiB).
> 
> [...]

But first, apply my patch.
Suddently the problem is much smaller.

-- 
MST

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Oct 21, 2014 at 03:29:14PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
[...]
>> >> My patch to get_maintainers.pl triggered a whole thread, while the
>> >> message I sent on MAINTAINERS coverage got just one reply so far, and
>> >> even that one's really just about get_maintainers.pl.  Disappointing.
>> >> Looks like we're still looking for an easy technical fix.  I doubt there
>> >> is one.
>> >
>> > At least for myself, that's because I'm Cc'd directly on the patch
>> > but not on the MAINTAINERS coverage mail.
>> > And that's ... because get_maintainers picks my mail from git?
>> >
>> > See how it's useful now?
>> 
>> Except that's not what happened.
>> 
>>     $ scripts/get_maintainer.pl --git-fallback -f scripts/get_maintainer.pl 
>> 
>> No output.  I picked you from git-log manually.
>
> Weird.
> It works for me:
>  ./scripts/get_maintainer.pl -f scripts/get_maintainer.pl 
> "Michael S. Tsirkin" <mst@redhat.com> (commit_signer:1/1=100%)
>
> Maybe --git-fallback is broken?

I tried on master (i.e. without my patch, clean tree, with and without
--git-fallback.  Just tried it again, same result.

> Another reason to defer this patch ...

Can't see why.  But I'm content to defer it until the patch traffic
triggered by this discussion quiets down, so we can reevaluate.

[...]

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

[Markus Armbruster]

"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.

This is what I'm proposing to do:

    $ scripts/get_maintainer.pl -f util/cutils.c
    No maintainers found.
    You may want to try --git-fallback to find recent contributors.
    Do not blindly cc: them on patches!  Use common sense.

Perhaps round off with a link to a Wiki page with additional advice on
how to find people to copy.

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

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 08:39:59AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Tue, Oct 21, 2014 at 03:29:14PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
> [...]
> >> >> My patch to get_maintainers.pl triggered a whole thread, while the
> >> >> message I sent on MAINTAINERS coverage got just one reply so far, and
> >> >> even that one's really just about get_maintainers.pl.  Disappointing.
> >> >> Looks like we're still looking for an easy technical fix.  I doubt there
> >> >> is one.
> >> >
> >> > At least for myself, that's because I'm Cc'd directly on the patch
> >> > but not on the MAINTAINERS coverage mail.
> >> > And that's ... because get_maintainers picks my mail from git?
> >> >
> >> > See how it's useful now?
> >> 
> >> Except that's not what happened.
> >> 
> >>     $ scripts/get_maintainer.pl --git-fallback -f scripts/get_maintainer.pl 
> >> 
> >> No output.  I picked you from git-log manually.
> >
> > Weird.
> > It works for me:
> >  ./scripts/get_maintainer.pl -f scripts/get_maintainer.pl 
> > "Michael S. Tsirkin" <mst@redhat.com> (commit_signer:1/1=100%)
> >
> > Maybe --git-fallback is broken?
> 
> I tried on master (i.e. without my patch, clean tree, with and without
> --git-fallback.  Just tried it again, same result.

Well ... I don't know why.
It's clearly a bug.
Different perl version? Different git version?

Can you try tracing it?
Does it exec git?

> > Another reason to defer this patch ...
> 
> Can't see why.

So we can debug and fix --git-fallback.


>  But I'm content to defer it until the patch traffic
> triggered by this discussion quiets down, so we can reevaluate.
> 
> [...]

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

[Michael S. Tsirkin]

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?


> This is what I'm proposing to do:
> 
>     $ scripts/get_maintainer.pl -f util/cutils.c
>     No maintainers found.
>     You may want to try --git-fallback to find recent contributors.
>     Do not blindly cc: them on patches!  Use common sense.
> 
> Perhaps round off with a link to a Wiki page with additional advice on
> how to find people to copy.

Let's start with that wiki page then.

-- 
MST

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

[Paolo Bonzini]

On 10/22/2014 09:12 AM, Michael S. Tsirkin wrote:
> >     $ 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?

Most likely, no one.  Just qemu-devel and/or qemu-trivial.

Paolo

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

[Markus Armbruster]

"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.

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".

[...]

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

[Thomas Huth]

On Wed, 22 Oct 2014 10:01:43 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Oct 22, 2014 at 08:39:59AM +0200, Markus Armbruster wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > 
> > > On Tue, Oct 21, 2014 at 03:29:14PM +0200, Markus Armbruster wrote:
> > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > >> 
> > >> > On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
> > [...]
> > >> >> My patch to get_maintainers.pl triggered a whole thread, while the
> > >> >> message I sent on MAINTAINERS coverage got just one reply so far, and
> > >> >> even that one's really just about get_maintainers.pl.  Disappointing.
> > >> >> Looks like we're still looking for an easy technical fix.  I doubt there
> > >> >> is one.
> > >> >
> > >> > At least for myself, that's because I'm Cc'd directly on the patch
> > >> > but not on the MAINTAINERS coverage mail.
> > >> > And that's ... because get_maintainers picks my mail from git?
> > >> >
> > >> > See how it's useful now?
> > >> 
> > >> Except that's not what happened.
> > >> 
> > >>     $ scripts/get_maintainer.pl --git-fallback -f scripts/get_maintainer.pl 
> > >> 
> > >> No output.  I picked you from git-log manually.
> > >
> > > Weird.
> > > It works for me:
> > >  ./scripts/get_maintainer.pl -f scripts/get_maintainer.pl 
> > > "Michael S. Tsirkin" <mst@redhat.com> (commit_signer:1/1=100%)
> > >
> > > Maybe --git-fallback is broken?
> > 
> > I tried on master (i.e. without my patch, clean tree, with and without
> > --git-fallback.  Just tried it again, same result.
> 
> Well ... I don't know why.
> It's clearly a bug.
> Different perl version? Different git version?
> 
> Can you try tracing it?
> Does it exec git?

I've got the same behaviour as Markus (i.e. no output), and I think
this is due to get_maintainer.pl using $email_git_since = "1-year-ago"
... since there wasn't a commit to that file in the last year, git-log
simply does not output any entry at all.
Do you have a non-upstream commit in your git tree that changes that
file? That would explain why you get some output here.

 Thomas

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

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 10:10:58AM +0200, Thomas Huth wrote:
> On Wed, 22 Oct 2014 10:01:43 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Oct 22, 2014 at 08:39:59AM +0200, Markus Armbruster wrote:
> > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > 
> > > > On Tue, Oct 21, 2014 at 03:29:14PM +0200, Markus Armbruster wrote:
> > > >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > >> 
> > > >> > On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote:
> > > [...]
> > > >> >> My patch to get_maintainers.pl triggered a whole thread, while the
> > > >> >> message I sent on MAINTAINERS coverage got just one reply so far, and
> > > >> >> even that one's really just about get_maintainers.pl.  Disappointing.
> > > >> >> Looks like we're still looking for an easy technical fix.  I doubt there
> > > >> >> is one.
> > > >> >
> > > >> > At least for myself, that's because I'm Cc'd directly on the patch
> > > >> > but not on the MAINTAINERS coverage mail.
> > > >> > And that's ... because get_maintainers picks my mail from git?
> > > >> >
> > > >> > See how it's useful now?
> > > >> 
> > > >> Except that's not what happened.
> > > >> 
> > > >>     $ scripts/get_maintainer.pl --git-fallback -f scripts/get_maintainer.pl 
> > > >> 
> > > >> No output.  I picked you from git-log manually.
> > > >
> > > > Weird.
> > > > It works for me:
> > > >  ./scripts/get_maintainer.pl -f scripts/get_maintainer.pl 
> > > > "Michael S. Tsirkin" <mst@redhat.com> (commit_signer:1/1=100%)
> > > >
> > > > Maybe --git-fallback is broken?
> > > 
> > > I tried on master (i.e. without my patch, clean tree, with and without
> > > --git-fallback.  Just tried it again, same result.
> > 
> > Well ... I don't know why.
> > It's clearly a bug.
> > Different perl version? Different git version?
> > 
> > Can you try tracing it?
> > Does it exec git?
> 
> I've got the same behaviour as Markus (i.e. no output), and I think
> this is due to get_maintainer.pl using $email_git_since = "1-year-ago"
> ... since there wasn't a commit to that file in the last year, git-log
> simply does not output any entry at all.
> Do you have a non-upstream commit in your git tree that changes that
> file? That would explain why you get some output here.
> 
>  Thomas

Aha. Right. Thanks!

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

[Michael S. Tsirkin]

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.

-- 
MST

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" <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