All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git <git@vger.kernel.org>,
	"Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH v1 15/19] config: add git_config_get_date_string() from gc.c
Date: Mon, 28 Nov 2016 08:56:06 -0800	[thread overview]
Message-ID: <xmqqeg1vlf15.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAP8UFD0Q4yTfnhLW6XbfqbxBvMc_QmZTEn4XJb-9fj6Uvq6hkw@mail.gmail.com> (Christian Couder's message of "Mon, 28 Nov 2016 17:19:17 +0100")

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

> 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?







  reply	other threads:[~2016-11-28 16:56 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-23  9:26 [PATCH v1 00/19] Add configuration options for split-index Christian Couder
2016-10-23  9:26 ` [PATCH v1 01/19] split-index: s/eith/with/ typo fix Christian Couder
2016-10-23  9:26 ` [PATCH v1 02/19] config: add git_config_get_split_index() Christian Couder
2016-10-23  9:26 ` [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions Christian Couder
2016-10-25  9:58   ` Duy Nguyen
2016-10-29 22:06     ` Christian Couder
2016-11-07 10:08       ` Duy Nguyen
2016-11-09  9:24         ` Duy Nguyen
2016-11-09 14:47           ` Christian Couder
2016-10-23  9:26 ` [PATCH v1 04/19] read-cache: add and then use tweak_split_index() Christian Couder
2016-10-23  9:26 ` [PATCH v1 05/19] update-index: warn in case of split-index incoherency Christian Couder
2016-10-25 10:00   ` Duy Nguyen
2016-10-29 22:19     ` Christian Couder
2016-11-01 19:05     ` Junio C Hamano
2016-11-01 23:00       ` Christian Couder
2016-11-02  1:37         ` Junio C Hamano
2016-10-23  9:26 ` [PATCH v1 06/19] t1700: add tests for core.splitIndex Christian Couder
2016-10-23  9:26 ` [PATCH v1 07/19] Documentation/config: add information " Christian Couder
2016-10-23  9:26 ` [PATCH v1 08/19] Documentation/git-update-index: talk about core.splitIndex config var Christian Couder
2016-10-23  9:26 ` [PATCH v1 09/19] config: add git_config_get_max_percent_split_change() Christian Couder
2016-10-25 10:06   ` Duy Nguyen
2016-10-29 22:24     ` Christian Couder
2016-11-01 19:13     ` Junio C Hamano
2016-11-05  0:27       ` Christian Couder
2016-10-23  9:26 ` [PATCH v1 10/19] read-cache: regenerate shared index if necessary Christian Couder
2016-10-23 16:07   ` Ramsay Jones
2016-10-29 22:40     ` Christian Couder
2016-10-25 10:16   ` Duy Nguyen
2016-10-29 22:58     ` Christian Couder
2016-10-23  9:26 ` [PATCH v1 11/19] t1700: add tests for splitIndex.maxPercentChange Christian Couder
2016-11-01 19:15   ` Junio C Hamano
2016-10-23  9:26 ` [PATCH v1 12/19] Documentation/config: add splitIndex.maxPercentChange Christian Couder
2016-11-01 19:19   ` Junio C Hamano
2016-11-05  0:45     ` Christian Couder
2016-11-06 17:16       ` Junio C Hamano
     [not found]         ` <CAP8UFD1YL+RgdqbV0V1OnC=sJHJFc_an02Q9JeDNapW+u1CZcA@mail.gmail.com>
2016-11-07  9:38           ` Duy Nguyen
2016-11-18 14:34             ` Christian Couder
2016-11-22 10:35               ` Duy Nguyen
2016-11-22 13:13                 ` Christian Couder
2016-11-22 13:20                   ` Duy Nguyen
2016-10-23  9:26 ` [PATCH v1 13/19] sha1_file: make check_and_freshen_file() non static Christian Couder
2016-10-23  9:26 ` [PATCH v1 14/19] read-cache: touch shared index files when used Christian Couder
2016-10-25 10:26   ` Duy Nguyen
2016-11-01 19:23     ` Junio C Hamano
2016-10-23  9:26 ` [PATCH v1 15/19] config: add git_config_get_date_string() from gc.c Christian Couder
2016-11-01 19:28   ` Junio C Hamano
2016-11-23 15:04     ` Christian Couder
2016-11-23 17:34       ` Junio C Hamano
2016-11-28 16:19         ` Christian Couder
2016-11-28 16:56           ` Junio C Hamano [this message]
2016-10-23  9:26 ` [PATCH v1 16/19] read-cache: unlink old sharedindex files Christian Couder
2016-10-25 10:43   ` Duy Nguyen
2016-10-27 10:25     ` Duy Nguyen
2016-10-27 12:14       ` Christian Couder
2016-10-27 16:13       ` Junio C Hamano
2016-10-29  3:30         ` Duy Nguyen
2016-10-23  9:26 ` [PATCH v1 17/19] t1700: test shared index file expiration Christian Couder
2016-10-23  9:26 ` [PATCH v1 18/19] Documentation/config: add splitIndex.sharedIndexExpire Christian Couder
2016-10-23  9:26 ` [PATCH v1 19/19] Documentation/git-update-index: explain splitIndex.* Christian Couder
2016-10-24 18:07 ` [PATCH v1 00/19] Add configuration options for split-index Junio C Hamano
2016-10-25  9:30   ` Duy Nguyen
2016-10-25 17:21     ` Junio C Hamano
2016-10-26  9:25       ` Duy Nguyen
2016-10-26 16:14         ` Junio C Hamano
2016-10-25 10:52 ` Duy Nguyen
2016-11-03 14:34   ` Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqeg1vlf15.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.