From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 19 Jun 2017 04:18:32 GMT
Subject: Re: [PATCH v2 3/4] sha1_file: consolidate storage-agnostic object fns
Message-ID: <xmqqr2ygvcdj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170617121934.a6onn7f2luhkuuym@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I actually think all of that infrastructure could become part of
> Jonathan's consolidated lookup, too. We would just need:
>
>   1. A QUICK flag to avoid re-reading objects/pack when we don't find
>      anything (which it looks like he already has).
>
>   2. A FRESHEN flag to update the mtime of any item that we do find.
>
> I suspect we may also need something like ONLY_LOOSE and ONLY_NONLOCAL
> to meet all the callers (e.g., has_loose_object_nonlocal). Those should
> be easy to implement, I'd think.

Ahh, that makes a lot more sense than my reading of the related
codepath.  Thanks for straightening me out.

> ...
> I think this patch might be a bit easier to review if it were broken
> down more in a sequence of:
>
>   1. Add features to the consolidated function to support everything
>      that function X supports.
>
>   2. Preparatory cleanup around X (e.g., pointing HAS_SHA1_QUICK at
>      OBJECT_INFO_QUICK).
>
>   3. Convert X to use the consolidated function.
>
>   4. Repeat for each X we wish to consolidate.
>
> That's going to end up with probably 12 patches instead of one, but I
> think it may be a lot easier to communicate the reason for the various
> design decisions.

True, too.  Thanks.