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

From: Emily Shaffer <[email-masked]>
Date: Wed, 24 Mar 2021 14:54:21 -0700

On Fri, Mar 12, 2021 at 10:08:04AM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> 
> On Thu, Mar 11 2021, Emily Shaffer wrote:
> 
> > 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