Re: [PATCH 03/16] refs: add methods for the ref iterators

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 4, 2016, 20:26 UTC
Message-ID: <xmqqegdxnlba.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAL=YDW=3JE==2s7Y3-fVRnTgaAzT674CAnypuXOKynpfD0q=Bg@mail.gmail.com>

Ronnie Sahlberg <sahlberg@google.com> writes:

Show 8 quoted lines

> Not commenting on whether this is the right direction or not. A global
> variable holding a methods table might not be most aesthetic design,
> but there are practicalities.
>
> However, that kind of change would change the function signatures for
> all public refs functions and probably most private refs functions as
> well and will likely have massive conflicts with almost any other
> patch that touches the refs code.

I do not think "the_backend" that is the default is wrong. Most callers want to interact with the single default backend that shows refs from the single default repository (i.e. ours), and having a set of functions that interact with that single default instance of the ref data without having to explicitly pass it as a parameter makes sense.

It is just like how we handle the in-core index. Most callers want to interact with the_index, the single default one, and we have a set of "cache" functions written back in Linus's days to access it without having to pass &the_index as the parameter. Later in April 2007 when I invented the support for multiple in-core index, we kept the traditional "works on the default instance" API in place but made them thin wrappers to the "works on the given instance", i.e. a set of "index" functions that was introduced at da94faf6 (Merge branch 'jc/the-index', 2007-04-24). Even after that change, most callers that work on the in-core index intract with "the_index", the single default instance, via the "cache" functions.

An API enhancement to allow us to handle refs in multiple repositories separately would be a very welcome move (it would get rid of the hacky interface for_each_ref_in_submodule(), for one thing), but even after that happens, I'd expect we would have most callers asking to interact with "the_backend", the single default instance.