Re: [PATCH 23/38] refs: make peel_ref() virtual

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Jun 9, 2016, 15:05 UTC
Message-ID: <575985C8.9040108@alum.mit.edu>
In-Reply-To: <xmqqtwh4rjdh.fsf@gitster.mtv.corp.google.com>

On 06/07/2016 07:36 PM, Junio C Hamano wrote:

Show 7 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> For now it only supports the main reference store.
> 
> Isn't this comment applicable to a handful of recent changes that
> made other things virtual, too?  Just wondering if I am missing
> something very special with the peel_ref() thing to single it out.

It is true that many (most?) virtual functions can currently only be used with the main reference store. That is likely to stay the case for reference-writing functions, because currently there isn't any code that wants to write references in other submodule. (Or maybe there is, but it's currently done by invoking a subprocess? I haven't actually looked.)

But peel_ref() is a read-only function, and it seems more plausible to implement it for other submodules. It just seems like work that can be put off. And there's the kindof ugly current_ref_iter hack that might go away if callers are rewritten to use ref_iterators, in which case implementing this method for other submodules would become even easier.

In other words, the main point of the comment is not "it only supports the main reference store", but rather "for now".

Michael