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

[Patrick Steinhardt <ps@pks.im>]

On Wed, Jan 21, 2026 at 07:15:16PM -0500, Taylor Blau wrote:
> 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?

As you say further down, it makes it easy to see that it's a function
that belongs to `struct odb_source_loose`. It immediately gives the
reader a sense what the main structure is it belongs to, thus gives
scope and makes LSPs work better because of the common prefix.

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

There will be more to come: iterating over objects with a prefix, for
example. In general, this series is taking a layered approach:

  - `odb_for_each_object()` is the high-level function that users should
    use if possible. It is part of the ODB layer and abstracts away
    details about the ODB sources.

  - `odb_source_for_each_object()` will be introduced in the next patch
    series. It allows the user to take an ODB source and iterate over
    its contained objects, regardless of what the backend is.

  - `odb_source_loose_for_each_object()` is the low-level implementation
    for one specific backend. We also have equivalent functions for the
    other backends, like for example for packed objects.

The longer the function name, the more specific the logic becomes. Sure,
eventually it becomes a mouthful, but ideally users wouldn't have to
ever interact with the low-level details at all.

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

So I would claim that this is even intentional. If a reader is not aware
what an ODB source is, then chances are high that using the function
that iterates through one specific source is the wrong thing to do. They
should rather use `odb_for_each_object()` in that case, which is the
higher-level interface that doesn't require the reader to know about ODB
sources in the first place.

I kind of see this as "guiding" the reader and giving them some hints
what the preferred interface is.

Patrick