Re: [PATCH v7 00/16] Parial clone part 3: clone, fetch, fetch-pack, upload-pack, and tests

From: Brandon Williams <bmwill@google.com>
Date: Dec 8, 2017, 22:30 UTC
Message-ID: <20171208223010.GF140529@google.com>
In-Reply-To: <20171208155851.855-1-git@jeffhostetler.com>

On 12/08, Jeff Hostetler wrote:

Show 13 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> This is V7 of part 3 of partial clone.  It builds upon V7 of part 2
> (which builds upon V6 of part 1).
> 
> This version adds additional tests, fixes test errors on the MAC version,
> and squashes some fixup commits.
> 
> It also restores functionality accidentally dropped from the V6 series
> for "git fetch" to automatically inherit the partial-clone filter-spec
> when appropriate.  This version extends the --no-filter argument to
> override this inheritance.
>

I just finished reading through parts 1-3. Overall I like the series. There are a few point's that I'm not a big fan of but i wasn't able to come up with a better alternative. One of these being the need for a global variable to tell the fetch-object logic to not go to the server to try and fetch a missing object.

One other thing i noticed was it looks like when you discover that you are missing a blob you you'll try to fault it in from the server without first checking its an object the server would even have. Shouldn't you first do a check to verify that the object in question is a promised object before you go out to contact the server to request it? You may have already ruled this out for some reason I'm not aware of (maybe its too costly to compute?).

-- 
Brandon Williams