> Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > > The patch itself look quite noisy, but in esssense, at the lowest > level we used to have a single format_packet() that was used to > write out normal payload and an error message prefixed with "ERR "; > now the users of the function are updated to call one of the two > helper functions, packet_writer_write() or packet_writer_error(). > Most of the patch noise comes from the helper functions at higher > levels getting updated to pass the packet_writer struct through the > callchain. > > Which makes tons of sense. Yes, that's right. Thanks. > > It will be convenient for a future patch if writing options > > (specifically, whether the written data is to be multiplexed) could be > > controlled from a single place, so create struct packet_writer to serve > > as that place, and modify upload-pack to use it. > > I've singled out "ERR " in my comment above, but this only refers to > "multiplexed". Are there reasons why we want multiplexing other > than the "are we sending payload, or an error message"? The main reason is to allow progress and keepalive messages at any time, actually. I'll add this paragraph at the start of the commit message: A future patch will allow the client to request multiplexing of the entire fetch response (and not only during packfile transmission), which in turn allows the server to send progress and keepalive messages at any time during the response.