Re: [PATCH v3 00/32] object-store: lookup_commit

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 3, 2018, 18:13 UTC
Message-ID: <xmqq8t6srxvq.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180629222004.GA197765@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 5 quoted lines

> I 100% think that we need to continue these refactorings with both the
> object store as well as with the_index (removing the index macros and
> removing the dependency on global state).  The whole compat macros most
> definitely was a failed experiment as we still haven't rid the code
> base of them yet.

These two are completely different things. You can call the compat macros a failed experiment only if two things hold true: (1) we want to force all callsites to explicitly pass the_index even when they always work on the primary in-core instance of istate and (2) they were invented primarily in the hope that their presense will help us achieve (1) sooner. And neither is true. The mechanism to allow multiple in-core istate were of course helpful in places like branch switching and merging, but we expected that the majority of then-existing code would be required to work on the primary in-core index, so s/foo_cache(/foo_index(&the_index/ conversion all over the place were noisier than it was worth. The compat macros were not introduced as an experiment to help wean ourselves off of the foo_cache API. It was a total opposite---it was a consideration on production code to help existing code keep working in backward compatible way. In short, they were neither failed nor experiment.

What you _could_ say about them, given that more code than before would benefit by learning to work on istate instances other than the default &the_index these days, is that they may have outlived their usefulness, though.