On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote: > >> Why does prepare_revision_walk() clear the list of pending objects at > >> all? Assuming the list is append-only then perhaps remembering the > >> last handled index would suffice. > > For that matter, why does it copy, instead of using revs->pending > directly? I do not think I can answer this, as I think the design > decisions led to this code predates me. Me too, then. :) I think part of that copy is that we're moving the items over to revs->commits, and dropping any non-commit objects. > In any case, it seems that the discussion has veered into an > interesting tangent but at this point it seems to me that it is not > likely to produce an immediate improvement over the posted patch. Fair enough. > Should we take the patch posted as-is and move forward? I suppose so. I don't really have anything against the patch. My main complaint was just that I don't think it's actually cleaning up the gross parts of the interface. It's just substituting one gross thing (a funny struct flag) for another (a special version of the prepare function that takes a funny out-parameter). -Peff