Re: [PATCH 5/6] hash_pos(): convert to oid_pos()

From: Jeff King <peff@peff.net>
Date: Jan 29, 2021, 01:18 UTC
Message-ID: <YBNifzRKYOjaZ4eV@coredump.intra.peff.net>
In-Reply-To: <YBM/NpLeS0uFzVb3@nand.local>

On Thu, Jan 28, 2021 at 05:48:22PM -0500, Taylor Blau wrote:

Show 15 quoted lines

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

Yeah. I'd be more inclined to do so if renames weren't still a minor pain when following history, etc. Getting rid of "sha1" crosses my bar of worth doing. The distinction between "hash" and "oid" here doesn't. (plus as you note, we'd really have to split it, which is even more annoying).

-Peff