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
