Re: [PATCH v1 15/19] config: add git_config_get_date_string() from gc.c

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 28, 2016, 16:56 UTC
Message-ID: <xmqqeg1vlf15.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAP8UFD0Q4yTfnhLW6XbfqbxBvMc_QmZTEn4XJb-9fj6Uvq6hkw@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 11 quoted lines

> What about something like this then:
>
> /* This dies if the configured or default date is in the future */
> extern int git_config_get_expiry_or_die(const char *key, const char **output);
>
> Also git_config_get_int(), git_config_get_bool() and probably other
> such functions are indirectly calling git_config_int() which die()s is
> the value is not a number, so it feels a bit strange to have only one
> function with a name that ends with "_or_die" when many other
> functions could die(). In fact it could give a false sense of security
> to those who just read cache.h.

It is half-a-good-point that existing functions would die and I agree with you that get_expiry_or_die can lose _or_die part.

But get_int() dying when it is fed something that is not "int" (or something that cannot be returned in "int") is one thing; get_date() being happy when it is fed a string that is a valid date 2015-01-01 but dying when fed another valid date 2018-01-01 is quite puzzling.

I think get_expiry() is OK.

Q: What's expiry?  How is it different from date
A: A past timestamp that is speifies the cutoff time for the purpose
   of expiring old stuff.  You can say "now" to expire everything
   you have.

is more pleasant and understandable conversation than

Q: Why can I feed only past date to get_date()?  Otherwise it dies.
A: ...?  The people who named the function didn't know what "date" was?