Re: [PATCH v5 2/3] advice: revamp advise API
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 25, 2020, 20:09 UTC
- Message-ID
- <xmqqblpmmntj.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20200225195648.GA212281@google.com>
Emily Shaffer <emilyshaffer@google.com> writes:
Show 21 quoted lines
>> ... Your get_config_value() would then become a mere lookup
>> in advice_setting[] array, e.g.
>>
>> int advice_enabled(unsigned advice_type)
>> {
>> static int initialized;
>>
>> if (!initialized) {
>> initialized = 1;
>> git_config(populate_advice_settings, NULL);
>> }
>> if (ARRAY_SIZE(advice_setting) <= advice_type)
>> BUG("OOB advice type requested???");
>> return !advice_setting[advice_type].disabled;
>> }
>>
>> with your "push-update-rejected has two names" twist added.
>
> I'm a little confused about the need to cache the result of
> git_config_get_bool() - isn't that a lookup from a hashmap which is
> already populated at setup time, and therefore inexpensive?Looking up from hashmap with a string key is always more expensive than indexing into a linear array with the array index. Also, the suggested arrangement makes the advice API implementation more self contained, I'd think.