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

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> On Fri, Jun 29, 2018 at 11:03 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > One technique these (not just this) recent efforts seem to be
>> > forgetting is to introduce "new" names that take a_repo and then
>> > make the existing one a thin wrapper that calls the new one with
>> > &the_repo as the argument.
>
> So you'd rather want to see it less invasive done similar to
> NO_THE_INDEX_COMPATIBILITY_MACROS ? Someone (jrnieder?)
> called that a failed experiment, as now we need to carry that baggage
> for quite some time and never cleaned up the started migration;
> only recently Duy started to kill off the_index, which would finish
> that migration?

I do not think it was a failed experiment at all.  In fact, most of
our code is still happy with the single in-core instance of
the_index, and I think it is a mistake to trying to use the variant
that take an istate instance as parameter just for the sake of it.

IOW, if there is a good concrete reason why it helps to teach the
set of functions involved in a callchain to operate on an in-core
instance of the index_state, passing an istate through the callchain
is a very welcome change.  If you do not do so, and then just
replace $foo_cache(...) with $foo_index(&the_index, ...), that is an
irritating and useless code churn that harms other topics in flight.