On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote: > "Michael S. Tsirkin" <m...@redhat.com> writes: > > > On Tue, Oct 21, 2014 at 11:31:12AM +0200, Markus Armbruster wrote: > >> "Michael S. Tsirkin" <m...@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 <m...@redhat.com> wrote: > >> >> > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote: > >> >> >> On 20 October 2014 10:19, Markus Armbruster <arm...@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 :)