Re: [PATCH v2 3/4] sha1_file: consolidate storage-agnostic object fns
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 19, 2017, 04:18 UTC
- Message-ID
- <xmqqr2ygvcdj.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170617121934.a6onn7f2luhkuuym@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> 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.
Show 17 quoted lines
> ... > 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.