On Fri, Aug 16, 2024 at 7:06 PM Jacob Keller [off-list ref] wrote:

In certain cases, a user may be generating a patch for an old commit
which now has an out-of-date author or other identity. For example,
consider a team member who contributes to an internal fork of a project,
and then later leaves the company.

It may be desired to submit this change upstream, but the author
identity now points to an invalid email address which will bounce. This
is likely to annoy users who respond to the email on the public mailing
list.

This can be manually corrected, but requires a bit of effort, as it may
require --suppress-cc or otherwise formatting a patch separately and
manually removing any unintended email addresses.

Git already has support for the mailmap, which allows mapping addresses
for old commits to new canonical names and addresses.

Teach git send-email the --mailmap option. When supplied, use git
check-mailmap (with the --no-brackets mode) as a final stage when
processing address lists. This will convert all addresses to their
canonical name and email according to the mailmap file.

A mailmap file can then be configured to point the invalid addresses
either to their current canonical email (if they still participate in
the open source project), or possibly to new owner within the company.

This enables the sender to avoid accidentally listing an invalid address
when sending such a change.

Nit: The final two paragraphs appear to repeat what was already stated
or implied earlier, thus don't seem to add any value to the commit
message.

Nit aside, similar to the question I asked about [1/2], are there
downsides to merely enabling this new behavior by default? It seems
like it would be generally desirable to have this translation happen
by default, so making everyone opt-in may be a disservice. On the
other hand, starting out with it disabled by default is understandable
as a cautious first step, though it might be nice to explain that in
the commit message. Similarly, one can imagine a world in which people
want to enable this and forget about it, thus would like it to be
controlled by configuration (though that can, of course, be left for a
future change).

For some reason, I found this logic more difficult to follow than
expected, possibly because it doesn't feel quite Perlish, or possibly
because in this codebase, we often take care of the easy cases first
and return early. Thus, I may have been expecting the above to be
written more along the lines of:

    sub mailmap_address_list {
        return @_ unless @_ && $mailmap;
        return Git::command('check-mailmap', '--no-brackets', @_);
    }

Of course, it's highly subjective and not at all worth a reroll.