Re: [PATCH v8 17/37] hooks: allow callers to capture output

On Fri, Mar 12, 2021 at 10:08:04AM +0100, Ævar Arnfjörð Bjarmason wrote:

On Thu, Mar 11 2021, Emily Shaffer wrote:

quoted

Some server-side hooks will require capturing output to send over
sideband instead of printing directly to stderr. Expose that capability.

So added here in 17/37 and not used until 30/37. As a point on
readability (this isn't the first such patch) I think it would be better
to just squash those together with some "since we now need access to
consume_sideband in hooks, do that ...".

Yeah. When I was putting together the series I had two thoughts on how
best to organize it:

1. Adding functionality just-in-time for the hook that needs it (like
you describe)
or
2. Implementing the whole utility, then doing hook conversions in a
separate chunk or series (what I went with).

I chose 2 for a couple reasons: that it would be easier for people who
just care "did a hook I use start working differently?" to review only
the second chunk of the change, and that it would be easier if we wanted
to adopt the library part into the codebase without converting the hooks
to use it (this was listed as a step in the design doc, but I think we
ended up abandoning it). The differentiation was certainly easier when I
had the two "chunks" separated into a part I and part II series, but
Junio asked me to combine them starting with this revision so it would
be easier to merge to 'seen' (as I understood it).

At this point, I'd prefer not to rearrange the series, though.

 - Emily