Re: [PATCH 2/2] hex: drop sha1_to_hex()

From: Jeff King <peff@peff.net>
Date: Nov 13, 2019, 01:15 UTC
Message-ID: <20191113011507.GC20431@sigill.intra.peff.net>
In-Reply-To: <xmqqbltgeg58.fsf@gitster-ct.c.googlers.com>

On Wed, Nov 13, 2019 at 10:09:23AM +0900, Junio C Hamano wrote:

Show 6 quoted lines

> Yes.  "binary hash" is about "unsigned char[]".  I think that's
> historical accident---we added "struct object_id *" variants without
> updating the comment.
> 
> Here is another try.  The "everybody uses oid_to_hex" in the log
> message has also been updated.

This looks good to me (and thank you for cleaning up my messy commit message).

Show 14 quoted lines

> 1:  8a030f1796 ! 1:  b19f3fe9dd hex: drop sha1_to_hex()
>     @@ Metadata
>       ## Commit message ##
>          hex: drop sha1_to_hex()
>      
>     -    There's only a single caller left of sha1_to_hex(), since everybody now
>     -    uses oid_to_hex() instead. This case is in the sha1dc wrapper, where we
>     -    print a hex sha1 when we find a collision. This one will always be sha1,
>     -    regardless of the current hash algorithm, so we can't use oid_to_hex()
>     -    here. In practice we'd probably not be running sha1 at all if it isn't
>     -    the current algorithm, but it's possible we might still occasionally
>     +    There's only a single caller left of sha1_to_hex(), since everybody
>     +    that has an object name in "unsigned char[]" now uses hash_to_hex()
>     +    instead.

That makes sense. I mentioned oid_to_hex() originally because most of the callers _did_ switch from sha1_to_hex(oid->hash) to oid_to_hex(oid). But that happened far enough in the past that the more interesting change is using the generic hash_to_hex().

-Peff