> not about this patch: can these return values from process_acks be made > into an enum with named enumerators? That would make what's happening > in the call site more obvious. That sounds reasonable to me. > > + cp trace /tmp/x && > > Leftover debugging line? Ah, yes. If Junio can't or won't do it locally then I'll send out another set with this changed. > > + test_i18ngrep "Total 3 " trace > > Clever. > > In some sense this is a fragile test, since the server could change > how it reports progress some day. Would it make sense (perhaps as a > followup patch) for this to use a trace2 log instead? For example, > if we turn on tracing in the server, then since 9ed8790282 > (pack-objects: write objects packed to trace2, 2019-04-11) it will > report how many objects were in the pack it wrote. Probably better to have tracing in the client and use that, but this requires us to add that tracing. But in general I agree. > After removing the "cp trace /tmp/x" line, > Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > > Thanks. Thanks for your review.
