Re: [PATCH] fetch: document that pruning happens before fetching

From: Jeff King <peff@peff.net>
Date: Jun 14, 2016, 10:22 UTC
Message-ID: <20160614102210.GA14357@sigill.intra.peff.net>
In-Reply-To: <CACsJy8Ds_DnBLJ0jx7Pp9EH1coG-xAtri4w4hh__=nigCmBbdA@mail.gmail.com>

On Tue, Jun 14, 2016 at 04:36:59PM +0700, Duy Nguyen wrote:

Show 29 quoted lines

> On Tue, Jun 14, 2016 at 6:58 AM, Jeff King <peff@peff.net> wrote:
> > This was changed in 10a6cc8 (fetch --prune: Run prune before
> > fetching, 2014-01-02), but it seems that nobody in that
> > discussion realized we were advertising the "after"
> > explicitly.
> 
> Ah... ok. Good to know it's moved up top on purpose because I almost
> tried to move it down :) It's irritating that current output looks
> like this
> 
> <delete ref>
> <delete ref>
> <delete ref>
> remote: <random progress lines>
> <update ref>
> <update ref>
> <update ref>
> 
> It probably looks better if we can move the <delete ref> part after
> "remote: ..." lines (iow still _after_ fetch, but _before_ ref
> updates), e.g.
> 
> remote: <random progress lines>
> <delete ref>
> <delete ref>
> <delete ref>
> <update ref>
> <update ref>
> <update ref>

I don't think it would be hard to do the deletion separate from the status-printing (in the worst case, you could simply queue the list of deleted refs and then print them later).

That might put the status lines for the deletions farther from any errors or warnings we print when doing the actual deletions, but in theory the error lines are self-contained and can stand on their own.

> If we do so, there's no need to update document. But I don't know,
> maybe it's not worth doing.

I think the documentation should be updated either way. This is not about the ordering in the status table, but rather about the order of the real operations. The user may care about that ordering if they want to know what races are possible, or what would happen if the packfile fetch failed (we'd already have deleted the old refs, but wouldn't fetch the new ones).

-Peff