On Thu, 25 Sep 2025, Junio C Hamano [off-list ref] wrote:

Adrian Ratiu [off-list ref] writes: 

quoted

From: Emily Shaffer <redacted> 

By using 'hook.h' for 'post-rewrite', we simplify hook 
invocations by 

This "By using 'hook.h" is somewhat a strange thing to say. 
<hook.h> has been in use by the file (evidenced by the fact that 
there is no new "#include <hook.h>" in the patch).  I haven't 
carefully read other steps in this series, but from my quick 
skimming of them, I got an impression that this comment may 
apply equally to other steps as well. 

What the commit does is not "use hook.h"; it is to replace a 
custom run-command call with a call to run_hooks_opt().  The 
shared API service function may happen to be declared in 
<hook.h>, that that is secondary piece of information. 

quoted

not needing to put together our own 'struct child_process'. 

Imperative?  I think just dropping "we" would be sufficient. 

quoted

The signal handling that's being removed by this commit now 
takes place in run-command.h:run_processes_parallel(), so it is 
OK to remove them here. 

Phrase it more positively, instead of "it is OK" (which sounds 
like it is also OK to leave it there).  Perhaps say something 
like: 

    Another benefit we gain from using run_hook_opt() instead of 
    a custom start_command()/finish_command() invocations is 
    that the hook API handles with sigpipe itself, so we no 
    longer need to toggle signals ourselves. 

or something like that, perhaps. 

Ack, will reword in v2. I actually inherited this message and thought
about rewording it as well :). Some others I already reworded in v1.