Re: [RFC PATCH] object-file API: add a format_loose_header() function

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 21, 2021, 01:42 UTC
Message-ID: <211221.86wnjyspd3.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqilviud6e.fsf@gitster.g>

On Mon, Dec 20 2021, Junio C Hamano wrote:

Show 13 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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*

Show 16 quoted lines

> 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.