Re: [PATCH v3 3/4] parse_commit(): handle broken whitespace-only timestamp

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

Phillip Wood <phillip.wood123@gmail.com> writes:

>> I did also allow "-" here, which may be controversial, as we don't
>> currently support negative timestamps. My reasoning was two-fold. One,
>> the design of parse_timestamp() is such that we should be able to easily
>> switch it to handling signed values, and this otherwise creates a
>> hard-to-find gotcha that anybody doing that work would get tripped up
>> on. And two, the status quo is that we currently parse them, though the
>> result of course ends up as a very large unsigned value (which is likely
>> to just get clamped to "0" for display anyway, since our date routines
>> can't handle it).
>
> I think this makes a good case for accepting '-'. The commit message
> is well explained as always :-) This all looks good to me apart from a
> query about one of the tests.

I agree.  I was somewhat surprised that the big comment before that
code did not mention it, but hopefully those who would be tempted to
remove the check for '-' would either be careful enough themselves
or be stopped by reviewers who are careful enough to go back to the
log message of the commit that added the check in the first place,
so it is OK.