On Thu, Apr 29 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> 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. The API is for our own internal use. So I figured it was better to leave the ones that aren't used elsewhere "static" for now, and if anyone needed them in the future that commit could remove the "static". > 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. Yes we could just drop this and inline the various "alloc", i.e. not this & similar in the future: - odb_commit = (struct commit *)create_object(r, &cur_oid, alloc_commit_node(r)); + odb_commit = create_commit(r, &cur_oid); It just seemed like a net improvement for maintenance/readability to have the simpler wrapper for the allocation / object creation v.s. the existing alloc_X_node() + cast.
