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

[Junio C Hamano <gitster@pobox.com>]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

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