Subject: Re: [PATCH v3 04/14] object-file: introduce function to iterate through objects

From: Taylor Blau <[email-masked]>
Date: Wed, 21 Jan 2026 19:15:16 -0500

On Wed, Jan 21, 2026 at 01:50:20PM +0100, Patrick Steinhardt wrote:
> Introduce a new function `odb_source_loose_for_each_object()` to plug
> this gap. This function doesn't take any data specific to loose objects,
> but instead it accepts a `struct object_info` that will be populated the
> exact same as if `odb_source_loose_read_object()` was called.

This may be a bit of a tangent, but I wonder if we are over-applying the
function prefixing convention.

In general I am really happy with this convention, and it yields
organized headers where functions are clearly grouped by what structure
they operate on. But I have noticed a handful of times where we replaced
a very concise function name with a longer prefixed version.

I think I don't have a clear sense of what the benefit of prefixing is
in this particular instance. Supposing for a moment that we don't have
an existing for_each_loose_object() function (which I think is the
end-state of this series). What does the name
"odb_source_loose_for_each_object()" convey that
"for_each_loose_object()" does not?

I think if there were multiple ways to iterate over loose objects, it
makes a lot of sense to prefix them such that they are grouped to avoid
mixing interfaces or using one API when you meant to call another. But
my understanding is that the intent here is to consolidate all of the
different ways to iterate over objects which live in different
odb_source implementations opaque to the caller. As a result, what other
way exists to iterate over loose objects?

Another aspect of this is how approachable the function is to newcomers.
On the one hand, I can see an argument that prefixing makes it clear
which functions belong together, and so if a newcomer is familiar with
the concept of ODB sources, then they should reasonably expect that a
function to iterate over loose objects would begin with "odb_source_".

But on the other hand, while a newcomer may be familiar with the basics
of Git's object model enough to understand the distinction between loose
and packed objects, they may not be familiar with the concept of an ODB
source. In that case, the prefix makes it somewhat more difficult to
find the right function to use.

I think there is a reasonable argument towards prefixing in the case
that we want to link against this function from outside of Git. But
AFAIK that is not likely to happen in the near future. So in the interim
I think we are left with function names which are a little more verbose
than the ones they are replacing without a clear benefit.

To be clear, I am generally in favor of this convention and have been
applying it myself especially when splitting out the repack builtin
implementation into their own compilation units. But I wonder if we
could relax the convention in cases like these without sacrificing
clarity/organization.

Thanks,
Taylor