public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Karthik Nayak <karthik.188@gmail.com>,
	Justin Tobler <jltobler@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 04/14] object-file: introduce function to iterate through objects
Date: Thu, 22 Jan 2026 19:01:15 -0500	[thread overview]
Message-ID: <aXK6S82kY8gwoEfQ@nand.local> (raw)
In-Reply-To: <aXHJEBY1FnbGRtzK@pks.im>

On Thu, Jan 22, 2026 at 07:52:00AM +0100, Patrick Steinhardt wrote:
> > 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.

Thanks for the extra information, this is definitely what I was missing.
If there are many ways to iterate over objects, then the naming scheme
above makes sense.

The point that I was trying to get across was that I think that the
convention of naming a function that does "foo" to a struct "S" as
"S_foo()" is great, but that we shouldn't apply that convention when
there is only one way to do "foo" in general.

For this particular case, I think I would have pushed back if you said
that `odb_for_each_object()` was the only function that we'd end up with
(i.e., there is no non-ODB way to do this, so for_each_object() is just
as descriptive IMO). But that's not the case, so I think the naming
scheme you have here makes sense.

Thanks,
Taylor

  reply	other threads:[~2026-01-23  0:01 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-15 11:04 [PATCH 00/14] odb: introduce `odb_for_each_object()` Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 01/14] odb: rename `FOR_EACH_OBJECT_*` flags Patrick Steinhardt
2026-01-15 18:00   ` Justin Tobler
2026-01-15 11:04 ` [PATCH 02/14] odb: fix flags parameter to be unsigned Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 03/14] object-file: extract function to read object info from path Patrick Steinhardt
2026-01-15 18:31   ` Justin Tobler
2026-01-16  7:03     ` Patrick Steinhardt
2026-01-20  9:09   ` Karthik Nayak
2026-01-15 11:04 ` [PATCH 04/14] object-file: introduce function to iterate through objects Patrick Steinhardt
2026-01-15 20:54   ` Justin Tobler
2026-01-16  7:03     ` Patrick Steinhardt
2026-01-20  9:16   ` Karthik Nayak
2026-01-15 11:04 ` [PATCH 05/14] packfile: extract function to iterate through objects of a store Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 06/14] packfile: introduce function to iterate through objects Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 07/14] odb: introduce `odb_for_each_object()` Patrick Steinhardt
2026-01-15 21:17   ` Justin Tobler
2026-01-16  7:03     ` Patrick Steinhardt
2026-01-16 17:46   ` Justin Tobler
2026-01-19  7:10     ` Patrick Steinhardt
2026-01-20  9:20   ` Karthik Nayak
2026-01-21  7:39     ` Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 08/14] builtin/fsck: refactor to use `odb_for_each_object()` Patrick Steinhardt
2026-01-15 21:24   ` Justin Tobler
2026-01-15 11:04 ` [PATCH 09/14] treewide: enumerate promisor objects via `odb_for_each_object()` Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 10/14] treewide: drop uses of `for_each_{loose,packed}_object()` Patrick Steinhardt
2026-01-15 21:44   ` Justin Tobler
2026-01-16  7:03     ` Patrick Steinhardt
2026-01-16 17:47       ` Justin Tobler
2026-01-19  7:10         ` Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 11/14] odb: introduce mtime fields for object info requests Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 12/14] builtin/pack-objects: use `packfile_store_for_each_object()` Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 13/14] reachable: convert to use `odb_for_each_object()` Patrick Steinhardt
2026-01-15 11:04 ` [PATCH 14/14] odb: drop unused `for_each_{loose,packed}_object()` functions Patrick Steinhardt
2026-01-15 13:50 ` [PATCH 00/14] odb: introduce `odb_for_each_object()` Junio C Hamano
2026-01-16  7:03   ` Patrick Steinhardt
2026-01-16 16:49     ` Junio C Hamano
2026-01-20 15:25 ` [PATCH v2 " Patrick Steinhardt
2026-01-20 15:25   ` [PATCH v2 01/14] odb: rename `FOR_EACH_OBJECT_*` flags Patrick Steinhardt
2026-01-20 15:25   ` [PATCH v2 02/14] odb: fix flags parameter to be unsigned Patrick Steinhardt
2026-01-20 15:25   ` [PATCH v2 03/14] object-file: extract function to read object info from path Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 04/14] object-file: introduce function to iterate through objects Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 05/14] packfile: extract function to iterate through objects of a store Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 06/14] packfile: introduce function to iterate through objects Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 07/14] odb: introduce `odb_for_each_object()` Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 08/14] builtin/fsck: refactor to use `odb_for_each_object()` Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 09/14] treewide: enumerate promisor objects via `odb_for_each_object()` Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 10/14] treewide: drop uses of `for_each_{loose,packed}_object()` Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 11/14] odb: introduce mtime fields for object info requests Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 12/14] builtin/pack-objects: use `packfile_store_for_each_object()` Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 13/14] reachable: convert to use `odb_for_each_object()` Patrick Steinhardt
2026-01-20 15:26   ` [PATCH v2 14/14] odb: drop unused `for_each_{loose,packed}_object()` functions Patrick Steinhardt
2026-01-21 12:50 ` [PATCH v3 00/14] odb: introduce `odb_for_each_object()` Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 01/14] odb: rename `FOR_EACH_OBJECT_*` flags Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 02/14] odb: fix flags parameter to be unsigned Patrick Steinhardt
2026-01-21 21:11     ` Jeff King
2026-01-22  0:00       ` Taylor Blau
2026-01-22 15:41         ` Junio C Hamano
2026-01-22 19:23           ` Jeff King
2026-01-23 10:57             ` Patrick Steinhardt
2026-01-26 22:32             ` Junio C Hamano
2026-01-22  6:50       ` Patrick Steinhardt
2026-01-22 23:44         ` Taylor Blau
2026-01-21 12:50   ` [PATCH v3 03/14] object-file: extract function to read object info from path Patrick Steinhardt
2026-01-22  0:04     ` Taylor Blau
2026-01-22  6:51       ` Patrick Steinhardt
2026-01-22 23:47         ` Taylor Blau
2026-01-21 12:50   ` [PATCH v3 04/14] object-file: introduce function to iterate through objects Patrick Steinhardt
2026-01-22  0:15     ` Taylor Blau
2026-01-22  6:52       ` Patrick Steinhardt
2026-01-23  0:01         ` Taylor Blau [this message]
2026-01-21 12:50   ` [PATCH v3 05/14] packfile: extract function to iterate through objects of a store Patrick Steinhardt
2026-01-22  1:37     ` Taylor Blau
2026-01-21 12:50   ` [PATCH v3 06/14] packfile: introduce function to iterate through objects Patrick Steinhardt
2026-01-23  0:06     ` Taylor Blau
2026-01-23  9:42       ` Patrick Steinhardt
2026-01-23  9:52         ` Chris Torek
2026-01-23 16:22           ` Junio C Hamano
2026-01-23 17:45             ` Taylor Blau
2026-01-21 12:50   ` [PATCH v3 07/14] odb: introduce `odb_for_each_object()` Patrick Steinhardt
2026-01-23  0:13     ` Taylor Blau
2026-01-21 12:50   ` [PATCH v3 08/14] builtin/fsck: refactor to use `odb_for_each_object()` Patrick Steinhardt
2026-01-23  0:32     ` Taylor Blau
2026-01-23  9:42       ` Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 09/14] treewide: enumerate promisor objects via `odb_for_each_object()` Patrick Steinhardt
2026-01-23  0:33     ` Taylor Blau
2026-01-21 12:50   ` [PATCH v3 10/14] treewide: drop uses of `for_each_{loose,packed}_object()` Patrick Steinhardt
2026-01-23  0:46     ` Taylor Blau
2026-01-23  9:43       ` Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 11/14] odb: introduce mtime fields for object info requests Patrick Steinhardt
2026-01-23  1:06     ` Taylor Blau
2026-01-23  9:43       ` Patrick Steinhardt
2026-01-23 17:48         ` Taylor Blau
2026-01-26  8:53           ` Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 12/14] builtin/pack-objects: use `packfile_store_for_each_object()` Patrick Steinhardt
2026-01-23  1:21     ` Taylor Blau
2026-01-23  9:43       ` Patrick Steinhardt
2026-01-23 18:35         ` Taylor Blau
2026-01-26  8:53           ` Patrick Steinhardt
2026-01-29 11:08             ` Jeff King
2026-01-30 12:57               ` Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 13/14] reachable: convert to use `odb_for_each_object()` Patrick Steinhardt
2026-01-21 12:50   ` [PATCH v3 14/14] odb: drop unused `for_each_{loose,packed}_object()` functions Patrick Steinhardt
2026-01-22  1:33   ` [PATCH v3 00/14] odb: introduce `odb_for_each_object()` Taylor Blau
2026-01-22 17:02     ` Junio C Hamano
2026-01-26  9:51 ` [PATCH v4 " Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 01/14] odb: rename `FOR_EACH_OBJECT_*` flags Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 02/14] odb: fix flags parameter to be unsigned Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 03/14] object-file: extract function to read object info from path Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 04/14] object-file: introduce function to iterate through objects Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 05/14] packfile: extract function to iterate through objects of a store Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 06/14] packfile: introduce function to iterate through objects Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 07/14] odb: introduce `odb_for_each_object()` Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 08/14] builtin/fsck: refactor to use `odb_for_each_object()` Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 09/14] treewide: enumerate promisor objects via `odb_for_each_object()` Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 10/14] treewide: drop uses of `for_each_{loose,packed}_object()` Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 11/14] odb: introduce mtime fields for object info requests Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 12/14] builtin/pack-objects: use `packfile_store_for_each_object()` Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 13/14] reachable: convert to use `odb_for_each_object()` Patrick Steinhardt
2026-01-26  9:51   ` [PATCH v4 14/14] odb: drop unused `for_each_{loose,packed}_object()` functions Patrick Steinhardt
2026-02-20 22:59   ` [PATCH v4 00/14] odb: introduce `odb_for_each_object()` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aXK6S82kY8gwoEfQ@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox