Subject: Re: [PATCH v2 11/19] pull: check if in unresolved merge state

From: Junio C Hamano <[email-masked]>
Date: Wed, 10 Jun 2015 10:14:16 -0700

Paul Tan <[email-masked]> writes:

> On Wed, Jun 10, 2015 at 10:38 PM, Junio C Hamano <[email-masked]> wrote:
>>> If you are going to do the git_config() call yourself, it might make
>>> more sense to define git_pull_config() callback and parse the pull.ff
>>> yourself, updating the use of the lazy git_config_get_value() API you
>>> introduced in patch 10/19.
>>>
>>> The above "might" is stronger than my usual "might"; I am undecided
>>> yet before reading the remainder of the series.
>>
>> Let me clarify the above with s/stronger/with much less certainty/;
>
> Uh, I have no idea what a "strong might" or a "less certain might" is. :-/
>
> Parsing all the config values with a git_config() callback function is
> certainly possible, but I was under the impression that we are moving
> towards migrating use of all the git_config() callback loops to the
> git_config_get_X() API.
>
> In this case though, we have to use git_config() to initialize the
> advice.resolveConflict config setting, but I don't see why it must be
> in conflict with the above goal.

I (or at least some part of me) actually view git_config_get_*() as
"if you are only going to peek a few variables, you do not have to
do the looping yourself" convenience, which leads me (or at least
that part of me) to say "if you are doing the looping anyway, you
may be better off picking up the variables you care about yourself
in that loop".

By calling git_config() before calling any git_config_get_*()
functions, you would be priming the cache layer with the entire
contents of the configuration files anyway, so later calls to
git_config_get_*() will read from that cache, so there is no
performance penalty in mixing the two methods to access
configuration data.  That is why I felt less certain that the
suggestion to stick to one method (instead of mixing the two) is a
good thing to do (hence "less certain 'might'").