Re: [PATCH 0/5] Start of a journey: drop NO_THE_INDEX_COMPATIBILITY_MACROS

[Stefan Beller <sbeller@google.com>]

On Mon, May 1, 2017 at 6:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This applies to origin/master.
>>
>> For better readability and understandability for newcomers it is a good idea
>> to not offer 2 APIs doing the same thing with on being the #define of the other.
>>
>> In the long run we may want to drop the macros guarded by
>> NO_THE_INDEX_COMPATIBILITY_MACROS. This converts a couple of them.
>
> Why?  Why should we keep typing &the_index, when most of the time we
> are given _the_ index and working on it?

As someone knowledgeable with the code base you know that the cache_*
and index_* functions only differ by an index argument. A newcomer may not
know this, so they wonder why we have (A) so many functions [and which is the
right function to use]; it is an issue of ease of use of the code base.

Anything you do In submodule land today needs to spawn new processes in
the submodule. This is cumbersome and not performant. So in the far future
we may want to have an abstraction of a repo (B), i.e. all repository state in
one struct/class. That way we can open a submodule in-process and perform
the required actions without spawning a process.

The road to (B) is a long road, but we have to art somewhere. And this seemed
like a good place by introducing a dedicated argument for the
repository. In a follow
up in the future we may want to replace &the_index by "the_main_repo.its_index"
and then could also run the commands on other (submodule) indexes. But more
importantly, all these commands would operate on a repository object.

In such a far future we would have functions like the cmd_* functions
that would take a repository object instead of doing its setup discovery
on their own.

Another reason may be its current velocity (or absence of it) w.r.t. to these
functions, such that fewer merge conflicts may arise.

---
This discussion is similar to the "free memory at the end of cmd_*" discussion,
as it aims to make code reusable, and accepting a minor drawback for it.
Typing "the_index" re-enforces the object thinking model and may have people
start on thinking if they would like to declare yet another global variable.

Thanks,
Stefan