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

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 22, 2026, 00:15 UTC
Message-ID: <aXFsFAV/1J9DLQRY@nand.local>
In-Reply-To: <20260121-pks-odb-for-each-object-v3-4-12c4dfd24227@pks.im>

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