Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 10, 2014, 17:29 UTC
Message-ID: <xmqqppg88d8x.fsf@gitster.dls.corp.google.com>
In-Reply-To: <53E4DF6D.8070904@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 13 quoted lines

> On 08/08/14 15:07, Tanay Abhra wrote:
> ...
>> (cc to Ramsay)
>> 
>> The discussion in both threads (v8 and v9), boils down to this,
>> is the `key_value_info` struct really required to be declared public or should be
>> just an implementation detail. I will give you the context,
>
> No, this is not the issue for me. The patch which introduces the
> struct in cache.h does not make use of that struct in any interface.
> It *is* an implementation detail of some code in config.c only.
>
> I do not know how that structure will be used in future patches. ;-)

It is debatable if it is a good API that tells the users "In the data I return to you, there is a structure hanging there with these two fields. Feel free to peek into it if you need what is recorded in them". But the contract between the the endgame "API" and its callers can include such a direct access, and then you can say that it is a part of the API, not just an implementation detail.

I think you and Tanay are both right (and I am wrong ;-).

You are right in that the "API" is giving more than the callers converted to it at this point in the series.

Tanay is right in that the way the struct will be used, which is illustrated by the example in the message you are responding to, should be in the part of this series that gives the implementation of the API before presenting the converted users, as the series deems it part of the API to let the users peek into the struct.

It may turn out that we have to abstract it further when we need a more elaborate data structure kept in the kv-info in the future. At that point it will become undesirable to keep giving the callers direct access to it, because direct struct access means that the particular aspect of the implementaiton detail will be cast in stone and would not allow to be replaced with some other representation that is more efficient for the implementation.

But as I said, what we see in this series can do for now. The future can come later ;-)