[Mesa-dev] [RFC PATCH] get_reviewer.pl: Delete

[Rob Clark]

On Wed, Feb 28, 2018 at 7:20 PM, Eric Anholt <eric at anholt.net> wrote:
> Rob Clark <robdclark at gmail.com> writes:
>
>> On Wed, Feb 28, 2018 at 4:09 PM, Eric Anholt <eric at anholt.net> wrote:
>>> Matt Turner <mattst88 at gmail.com> writes:
>>>
>>>> I find this script *really* annoying. Getting Cc'd on a random sample of
>>>> a series is doing it wrong. Cc lists of 14 people is doing it wrong.
>>>>
>>>> Let's start the negotiation with "delete this script" and see if anyone
>>>> can come up with a way of making this not so stupid.
>>>
>>> Patch submission done well IMO would be something like github PRs, with
>>> a bot that auto-tags people based on a MAINTAINERS-style opt-in for
>>> reviewing certain paths within the tree.
>>>
>>> I don't think get_reviewers.pl improves the git-send-email situation
>>> compared to maintainers needing to manage email filters.  It's not
>>> consistent, so you need to indoctrinate new submitters (more barriers to
>>> entry!) and maintainers need to maintain their mail filters anyway.
>>
>> Hmm, you have mail filters that parse paths in git patches?
>
> No, I actually skim all the patches.  But it's what you'd have to do if
> you want to actually see all patches to your area and not actually skim
> all the patches yourself, since get_reviewer.pl isn't (and won't ever
> be) used universally.

Well, not-withstanding a switch to a non-email based process, I've
found get_reviewer.pl semi-useful.. sometimes it has flagged someone I
wouldn't have thought to CC who provided useful review comments and
(presumably?) it has spammed me on things I wouldn't have noticed
otherwise..

That said, I'm not defending it's implementation.. it does seem to
come up with some crazy recommendations.  I use it in '-i' interactive
mode and try to trim off some of the more implausible suggestions it
makes (although tbh when it suggests someone who is a frequent mesa
contributor like Matt I tend to assume that was a correct choice and
not pay more attention).

I suppose part of my opinion on it is based on tending to jump between
kernel and mesa and other random things and not being very good at
paying attention to other lists when I am wearing the wrong hat unless
I get cc'd on something.. so I tend not to assume that just because a
patch hit list, that the relevant people will notice it.  And part is
because it seems to work reasonably well for kernel (although
admittedly on the kernel side there is the additional problem of
CC'ing the correct mailing list(s) which isn't a problem for mesa).

IMO changing it to only consider MAINTAINERS and not try to be clever
about git-blame and reviewer history, would be a reasonable solution
(at least given the current email based process).  At least then it is
an opt-in thing.  I'd at least like to be able to opt-in to being CC'd
on patches that touch certain parts of the code.

BR,
-R