Re: [PATCH v2 05/10] {commit,tree,blob,tag}.c: add a create_{commit,tree,blob,tag}()

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 29, 2021, 04:45 UTC
Message-ID: <xmqqa6ph8zsc.fsf@gitster.g>
In-Reply-To: <patch-05.10-5213d500b9-20210420T124428Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 6 quoted lines

> Add a create_*() function for our built-in types as a handy but
> trivial wrapper around their calls to create_object().
>
> This allows for slightly simplifying code added in
> 96af91d410c (commit-graph: verify objects exist, 2018-06-27). The
> remaining three functions are added for consistency for now.

"for now" puzzles me. As file-scope static functions, they do not hurt all that much, but on the other hand, having to say "create_object(r, oid, alloc_blob_node(r))" is not hurting at all.

The worst part of this "consistency" is that callers cannot call create_blob() because it is not external, even though they learn create_commit() as a handy way to use the create_object() API, which is not consistent at all.

And since most callers should be calling lookup_blob() etc., and should not be calling create_blob(), we shouldn't tempt people to push for making them externally available.

Which in turn makes me wonder if the use of create_object() added to the commit-graph.c was a good idea to begin with.