Re: [PATCH v2] archive-tar: fix a sparse 'constant too large' warning

Ramsay Jones [off-list ref] writes:

Yeah, I had a similar comment in the commit message (but much more
verbose than your concise addition above), but I edited it several
times, without finding a wording that I liked. I eventually removed
it, because it didn't really add any value. :(

I tend to agree that the proposed additional comment does not add
much value.  It assures the readers that we (at the time of applying
this patch) know that the earlier use of ULL was not done with a
good reason but was merely an accident, and strengthens the claim
that this is a good change, but the correctness of the change is
already obvious, and the readers would understand without being
explained where the incorrectness we have to fix with this patch
came from, I would think.
.

Hi Junio,

On Wed, 10 May 2017, Junio C Hamano wrote:

Ramsay Jones [off-list ref] writes:

quoted

Yeah, I had a similar comment in the commit message (but much more
verbose than your concise addition above), but I edited it several
times, without finding a wording that I liked. I eventually removed
it, because it didn't really add any value. :(

I tend to agree that the proposed additional comment does not add much
value.  It assures the readers that we (at the time of applying this
patch) know that the earlier use of ULL was not done with a good reason
but was merely an accident, and strengthens the claim that this is a
good change, but the correctness of the change is already obvious, and
the readers would understand without being explained where the
incorrectness we have to fix with this patch came from, I would think.

Future me would find that comment in the commit message very clarifying,
though: why was that code there? Ah, that's why.

Now I have to dig through the mailing list to find out.

Ciao,
Dscho