Re: [PATCH] scripts/get_maintainer.pl: Deprioritize old Fixes: addresses

[Joe Perches <joe@perches.com>]

On Mon, 2020-01-27 at 09:50 -0800, Douglas Anderson wrote:
> Recently, I found that get_maintainer was causing me to send emails to
> the old addresses for maintainers.  Since I usually just trust the
> output of get_maintainer to know the right email address, I didn't
> even look carefully and fired off two patch series that went to the
> wrong place.  Oops.
> 
> The problem was introduced recently when trying to add signatures from
> Fixes.  The problem was that these email addresses were added too
> early in the process of compiling our list of places to send.  Things
> added to the list earlier are considered more canonical and when we
> later added maintainer entries we ended up deduplicating to the old
> address.
> 
> Here are two examples using mainline commits (to make it easier to
> replicate) for the two maintainers that I messed up recently:
> 
> $ git format-patch d8549bcd0529~..d8549bcd0529
> $ ./scripts/get_maintainer.pl 0001-clk-Add-clk_hw*.patch | grep Boyd
> Stephen Boyd <sboyd@codeaurora.org>...
> 
> $ git format-patch 6d1238aa3395~..6d1238aa3395
> $ ./scripts/get_maintainer.pl 0001-arm64-dts-qcom-qcs404*.patch | grep Andy
> Andy Gross <andy.gross@linaro.org>
> 
> Let's move the adding of addresses from Fixes: to the end since the
> email addresses from these are much more likely to be older.
> 
> After this patch the above examples get the right addresses for the
> two examples.
> 
> Fixes: 2f5bd343694e ("scripts/get_maintainer.pl: add signatures from Fixes: <badcommit> lines in commit message")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> I'm no expert at this script and no expert at Perl.  If moving this
> call like I'm doing is totally stupid then please let me know what a
> more proper fix is.  Thanks!

No, it's correct and I looked at exactly this
change after this email exchange but I haven't
posted it because there might a be better way to
restructure this to allow the deduplication to
be done properly.

https://lore.kernel.org/lkml/b72846874d5ee367bf86e787ca2152f90d814a34.camel@perches.com/

Anyway, for now, it's likely good enough so:

Acked-by: Joe Perches <joe@perches.com>

>  scripts/get_maintainer.pl | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 34085d146fa2..7a228681f89f 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -932,10 +932,6 @@ sub get_maintainers {
>  	}
>      }
>  
> -    foreach my $fix (@fixes) {
> -	vcs_add_commit_signers($fix, "blamed_fixes");
> -    }
> -
>      foreach my $email (@email_to, @list_to) {
>  	$email->[0] = deduplicate_email($email->[0]);
>      }
> @@ -974,6 +970,10 @@ sub get_maintainers {
>  	}
>      }
>  
> +    foreach my $fix (@fixes) {
> +	vcs_add_commit_signers($fix, "blamed_fixes");
> +    }
> +
>      my @to = ();
>      if ($email || $email_list) {
>  	if ($email) {