Re: [PATCH 1/4] log-tree: create for_each_decoration()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 3, 2022, 18:40 UTC
Message-ID: <xmqqv8thfv56.fsf@gitster.g>
In-Reply-To: <55d7146d-0fc4-3c29-b9a4-72f5ff856255@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 15 quoted lines

>>> +	for_each_decoration(commit, append_decoration, &ctx);
>> 
>> The function for_each_decoration() that does not take decoration but
>> a commit felt iffy,...
>> ...
>> 	for_each_decoration(get_name_decoration(&commit->object),
>>         		    do_whatever_they_need_to_do, &ctx);
>> 
>> so, we may want to revisit this after we finish reading the series
>> through.  Making the iterator take name_decoration does not look
>> too bad.
>
> You are right that this would work, but it's a bit messier. We
> could go this route to avoid the duplicate get_name_decoration()
> calls in format_decorations_extended().

Having a convenience function that takes a commit and let you walk over its decoration is OK, but calling it for_each_decoration() bothers me somewhat (which is where my comment started from after all).

Hopefully it will stay unambiguous, and it will stay to be unnecessary to name it for_each_decoration_for_commit(), as I do not think we'd add "decoration" to things that are not commits (or if we added one, we probably will call it differently from "decoration").

So, OK.