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

From: Christian Couder <christian.couder@gmail.com>
Date: Nov 28, 2016, 16:19 UTC
Message-ID: <CAP8UFD0Q4yTfnhLW6XbfqbxBvMc_QmZTEn4XJb-9fj6Uvq6hkw@mail.gmail.com>
In-Reply-To: <xmqqlgwam76c.fsf@gitster.mtv.corp.google.com>

On Wed, Nov 23, 2016 at 6:34 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> Ok it will appear like this in cache.h:
>>
>> /* This dies if the configured or default date is in the future */
>> extern int git_config_get_expire_date_string(const char *key, const
>> char **output);
>
> Those who imitate existing callsites never read comments, and you
> need to spend effort to get the name right to protect the codebase
> from them.
>
> "get-expiry" may be shorter.  Neither still does not say it will
> die, though.

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.