Re: [PATCH v2 0/4] Add support for mailmap in cat-file

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2022, 22:06 UTC
Message-ID: <xmqqsfncy3w1.fsf@gitster.g>
In-Reply-To: <20220707161554.6900-1-siddharthasthana31@gmail.com>

Siddharth Asthana <siddharthasthana31@gmail.com> writes:

Show 12 quoted lines

> Changes in v2:
> - The commit_rewrite_person() has been improved by restricting it to
>   traverse only the header part of the object buffers.
> - The callers of commit_rewrite_person() now don't require to call it
>   multiple times for different headers. They can pass an array of
>   headers and commit_rewrite_person() replaces idents only on those
>   headers.
> - commit_rewrite_person() has been renamed to a suitable name which
>   expresses its functionality clearly.
> - More tests have been added to test the --[no-]-use-mailmap option for
>   the tag objects.
> - Redundant operations from the tests have been removed.

I agree with the general direction and the implementation strategy. I've noticed a few decl-after-statement and also at least one public helper function that does not need to be public. Are you building with "make DEVELOPER=YesPlease"? It enables -pedantic and -Werror, -Wdeclaration-after-statement, among other options (see the config.mak.dev file for the complete list) to help you catch these locally before sendign your patches to the list.

Thanks.