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