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

[Markus Armbruster]

"Michael S. Tsirkin" <address@hidden> writes:

> On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <address@hidden> writes:
>> 
>> > On Mon, Oct 20, 2014 at 03:19:52PM +0100, Peter Maydell wrote:
>> >> On 20 October 2014 15:15, Michael S. Tsirkin <address@hidden> wrote:
>> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote:
>> >> >> On 20 October 2014 10:19, Markus Armbruster <address@hidden> 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.