RE: [PATCH 1/2] devtools/mailmap_ctl: script to work with mailmap

[Marat Khalili <marat.khalili@huawei.com>]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday 8 August 2025 20:59
> To: Marat Khalili <marat.khalili@huawei.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 1/2] devtools/mailmap_ctl: script to work with mailmap

> > > +def check_mailmap(mailmap_path, _):
> > > +    """Check that mailmap entries are correctly sorted and formatted."""
> >
> > As noted above, it will not fail if some entries are incorrectly formatted.
> >
> > Also, we could probably check for duplicates.
> >
> Yes, but I will leave this for future work, as I don't believe it's a
> problem we currently have with our mailmap file.

Completely agree that we don't have to solve all problems at once, especially those that don't exist :)

> > > +    # Check if entry already exists, checking email2 only if it's specified
> > > +    if (
> > > +        not new_entry.email2
> > > +        and any(e.name == new_entry.name and e.email1 == new_entry.email1 for e in entries)
> > > +    ) or any(
> >
> > This will usually trigger even when `not new_entry.email2`.
> >
> Can you clarify this comment? Is there something I need to fix here?

When `not new_entry.email2` and there are no entries with `name` and `email1` matching new entry the interpreter will execute second `any` creating another iterator and running through all entries again this time looking for entries with `name`, `email1` and `email2` matching new entry, and never find anything since looser condition checking only `name` and `email1` failed earlier. Nothing would fail or behave differently now, it's just unnecessary work.

It's more like code smell than actual problem, so not worth another iteration unless you feel like it.

> > > +    parser.add_argument("entry", nargs="?", help='Entry to add. Format: "Name <email@domain.com>"')
> >
> > Secondary email is not mentioned. Actually, if I want to add a secondary email when I already have primary,
> what do I do?
> >
> You hand-edit for now! :-)
> That is something that I think we should add in future, but I'm keeping it
> simple for now. [For most of the patch application that I do to my tree,
> it's only adding completely new entries to mailmap, so getting that working
> was my primary concern]

Yeah, I understand, no problem leaving this out for now.