Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

[Junio C Hamano <gitster@pobox.com>]

Miklos Vajna <vmiklos@frugalware.org> writes:

> - removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()

Wait, a, minute, please.  Not so fast.  I did not say "I think this is
wrong, please fix it".

I only asked why it is so.

Care to explain why it was thought to be necessary, and why you now think
it is unnecessary?

This comment is not just for you, but when I ask why it is so, I expect
that the contributor would do the homework and present an argument to
defend what was submit (or "it was wrong for such and such reasons so
let's do it differently like this because this is better for such and such
reasons").  The original contributor has thought about the issues much
longer and deeper than I, who only just look at the email and kibitz,
after the contribution was perfected into a patch form.  There must have
been a lot more thinking than what can be seen in the submitted material
in the contributor's head.  I want to see that in the open.  Please do not
change your mind and retract your change without explaining why.

The argument to respond to such "why is it so?" question would perhaps
look like this for this case.

(begin example #1)

Well, I just cut and pasted from existing code.  Because all these hooks
redirect their stdout to stderr, I think it is consistent to do so with
the new hook.

(example #1 ends here)

(begin example #2)

"git grep -n -e stdout_to_stderr" followed by "git blame" reveals the
following pattern:

 - builtin-checkout.c:41
   782c2d6 (Build in checkout, 2008-02-07)

 - builtin-commit:382
   f5bbc32 (Port git commit to C., 2007-11-08)

 - help.c:62
   6494998 (help: add "man.viewer" config var to use "woman" or "konqueror", 2008-03-07)

 - receive-pack.c:120,157
   f43cd49 (Change {pre,post}-receive hooks to use stdin, 2007-03-10)
   1d9e8b5 (Split back out update_hook handling in receive-pack, 2007-03-10)

 - transport.c:165,218,297
   cd547b4 (fetch/push: readd rsync support, 2007-10-01)

I think all of these are the result of cut&paste from the original one in
receive-pack.c's hook handling.

The ones in the protocol handler may not write to the standard output,
because the other end does not expect random diagnostic string in the
protocol message at the point these hooks are called.  But the other ones
could go either way.

However, because existing hooks all show their script output to stderr, it
would make sense for this new pre-auto-gc hook to do so as well from the
consistency viewpoint.  So use of stdout_to_stderr is the right thing to
do here.

(example #2 ends here)