On Thu, Jan 28, 2021 at 01:19:42AM -0500, Jeff King wrote:
> All of our callers are actually looking up an object_id, not a bare
> hash. Likewise, the arrays they are looking in are actual arrays of
> object_id (not just raw bytes of hashes, as we might find in a pack
> .idx; those are handled by bsearch_hash()).
>
> Using an object_id gives us more type safety, and makes the callers
> slightly shorter. It also gets rid of the word "sha1" from several
> access functions, though we could obviously also rename those with
> s/sha1/hash/.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> If we don't want to make this change, I think it's still worth sweeping
> through the callers and changing the names of their access functions.
>
> builtin/name-rev.c | 8 ++++----
> commit-graph.c | 28 ++++++++++++++--------------
> commit.c | 10 +++++-----
> hash-lookup.c | 18 +++++++++---------
> hash-lookup.h | 10 +++++-----
I wondered briefly if we should rename this to oid-lookup.{c,h}, but I
think the answer is "no", since we still have bsearch_hash() in this
header.
Probably a single header with "hash" in the name is better than two
headers each containing a single function (and one containing an
additional typedef).
So, I think that what you did here is good.
Thanks,
Taylor
