On Mon, Dec 20 2021, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> Add a convenience function to wrap the xsnprintf() command that
>> generates loose object headers. This code was copy/pasted in various
>> parts of the codebase, let's define it in one place and re-use it from
>> there.
>> ...
>> +/**
>> + * format_loose_header() is a thin wrapper around s xsnprintf() that
>
> The name should have "object" somewhere in it. Not all readers can
> be expected to know that you meant "loose" to be an acceptable short
> hand for "loose object".
*nod*
> That nit aside, I think it is a good idea to give people a common
> helper function to call. I am undecided if it is a good idea to
> make it take enum or "const char *"; most everybody should be able
> to say
>
> format_object_header(type_name(OBJ_COMMIT), ...)
>
> just fine, so two variants might be overkill, just to allow
>
> format_object_header(OBJ_COMMIT, ...)
>
> and to forbid
>
> format_object_header("connit", ...)
>
> I dunno.
Ultimately only a single API caller in hash-object.c really cares about
something else than the enum.
I've got some patches locally to convert e.g. write_object_file() to use
the enum, and it removes the need for some callers to convert enum to
char *, only to have other things convert it back.
So I think for any new APIs it makes sense to work towards sidelining
the hash-object.c --literally caller.
