Re: [PATCH 0/2] Slightly prettier reflog message from checkout

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2013, 01:24 UTC
Message-ID: <7vmwqqkftv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1371317906-3991-1-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 12 quoted lines

> [1/2] is important.  [2/2] is a minor prettification, that wouldn't
> have been possible without [1/2].
>
> Thanks.
>
> Ramkumar Ramachandra (2):
>   sha1_name: stop hard-coding 40-character hex checks
>   checkout: do not write full sha1 to reflog
>
>  builtin/checkout.c | 2 +-
>  sha1_name.c        | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)

I view the two codepaths touched by these patches the other way around.

An abbreviated unique SHA-1 you have today may not be unique tomorrow. There is no reason to deliberately lose information (e.g. by using "Then, instead of the absolute minimum, let's record a bit more bytes" heuristics) when we record. The reflog recording code in checkout writes full 40-characters on purpose and there is no reason not to do so (i.e. the codepath that is the topic of 2/2).

That is a more important design decision between the two codepaths.

Once we accept that design principle of not losing information when we do not have to, it naturally follows that the writing side should write full 40-hex, and also the reading side (i.e. the codepath that is the topic of 1/2) should make sure that it reads 40-hex and nothing else. This also reduces the risk of a funny branch name that consists only of [0-9a-f] getting mistaken as an object name, but that is not the primary point.

So I am fairly strongly negative on both changes.