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

Stefan Beller [off-list ref] writes:

On Fri, Jun 29, 2018 at 11:03 AM Junio C Hamano [off-list ref] wrote:

quoted

Junio C Hamano [off-list ref] writes:

quoted

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.