Re: [PATCH 2/3] ident: rename commit_rewrite_person() to rewrite_ident_line()
From: Christian Couder <hidden>
Date: 2022-06-30 16:56:07
Hi Phillip, On Thu, Jun 30, 2022 at 5:33 PM Phillip Wood [off-list ref] wrote:
On 30/06/2022 15:24, Siddharth Asthana wrote:quoted
We will be using commit_rewrite_person() in git-cat-file to rewrite ident line in commit/tag object buffers.
s/line/lines/
quoted
Following are the reason for renaming commit_rewrite_person(): - the function can be used not only on a commit buffer, but also on a tag object buffer, so having "commit" in its name is misleading. - the function works on the ident line in the commit/tag object buffers, just like "split_ident_line()". Since these functions are related they should have similar terms for uniformity.I'm afraid I'm not sure about this change as the interface for split_ident_line() and commit_rewrite_person() are not uniform. split_ident_line() takes a pointer to the beginning of the name in an ident line and a length. commit_rewrite_person() takes the whole commit buffer and searches for the ident line based on the argument "what". I agree that having commit in the name of the function is confusing when it can be used for a tag, but having line in the name when it takes a whole buffer is also confusing.
It takes a whole buffer but it rewrites only ident lines, so maybe "rewrite_ident_lines()" (so with "lines" instead of "line").
Maybe buffer_rewrite_person() or something like that would be clearer?
I don't think "person" is better than "ident" for this, and I think it's better to use the same name for it in split_ident_line() and the function we are renaming. It's true that we are not rewriting the date, so maybe "rewrite_person_in_ident_lines()". Thanks, Christian.