From: Jeff King <peff@peff.net>
Date: Wed, 05 Jul 2017 21:31:35 GMT
Subject: Re: [PATCH 5/6] rev-list: check reflog_info before showing usage
Message-ID: <20170705213135.wlijd5kum5lyh752@sigill.intra.peff.net>
In-Reply-To: <xmqqd19eiwpk.fsf@gitster.mtv.corp.google.com>

On Wed, Jul 05, 2017 at 11:07:35AM -0700, Junio C Hamano wrote:

> > This is actually the main "gotcha" I'm worried about with this series.
> > I'm not sure if any other code would care about seeing the pending items
> > in revs->commits. I still think the series is the right direction; if
> > there is such a place, we'd want to teach it to handle reflog walking in
> > a similar way, too.
> 
> Ah, very good thinking---when I was following along your drafts to
> bypass the revs.commits queue for reflog walking, I didn't think of
> this one.  
> 
> Perhaps "!revs.commits && reflog_walk_empty(revs.reflog_info)" may
> want to become a macro "walk_finished(&revs)" or something to
> replace existing !revs.commits that is not accompanied by the check
> on .reflog_info field?

Yeah, I had the same thought. But I don't think it's really worth adding
a helper if we don't end up with another call-site. So I was stalling to
see if that happened.

(And just for reference in case we do end up adding it, it's not so much
walk_finished() as walk_empty_in_the_first_place()).

-Peff