Re: [PATCH 2/2] fetch-pack: respect --no-update-shallow in v2

From: Jeff King <peff@peff.net>
Date: Mar 26, 2019, 18:18 UTC
Message-ID: <20190326181806.GA24105@sigill.intra.peff.net>
In-Reply-To: <20190326173706.175638-1-jonathantanmy@google.com>

On Tue, Mar 26, 2019 at 10:37:06AM -0700, Jonathan Tan wrote:

Show 14 quoted lines

> > Perhaps it's worth passing down the shallows array we get from the
> > caller of fetch_pack(). Something like the patch below (I think it is
> > never NULL, which means in your patch 1 you can simplify the conditional
> > for the BUG).
> 
> [snip patch]
> 
> You're right that it is never NULL - I have removed that check. As for
> passing down the shallows array that we get from the caller of
> fetch_pack(), that would get confusing because we end up modifying the
> shallows array in some code paths, and the transport is sometimes reused
> (for example, when backfilling tags). I have instead made a
> shallows_scratch variable in fetch_pack(), and made it pass it down
> (like in the diff you provided).

Yeah, I confess to having spent quite a few minutes trying to figure out the difference between "shallows" and "shallow_info", whether one wrote into the other, and who was responsible for filling each in. So I will not complain if you have a way of writing it that is less confusing. :)

-Peff