From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 14 Jan 2019 20:07:31 GMT
Subject: Re: [PATCH 1/4] pkt-line: introduce struct packet_writer
Message-ID: <xmqqbm4juhoc.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <9b37e68e2f0d83ea133f6662efb780466c518170.1547244620.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> 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.

> 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"?

> Currently, it only stores the output fd, but a subsequent patch will (as
> described above) introduce an option to determine if the written data is
> to be multiplexed.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---