Re: [PATCH 4/4] t4300 (rebase): don't unnecessarily set GIT_TRACE

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: May 10, 2013, 18:06 UTC
Message-ID: <CALkWK0kTvwxMFGUqH0wKNVpLg-qQQbN+vOcAxiEwmOeDe=87jA@mail.gmail.com>
In-Reply-To: <7vk3n6206h.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 6 quoted lines

> To me, most of them look like responses to valid issues, and that
> holds true even for [PATCH 1/4].  Even though your response may have
> been an incorrect one, the issue that triggered the response is
> still valid---the setting of these variables without explanation
> invites curiosities and a mistake similar to what you made in that
> patch.

I like comments, but lately I've seen nothing but reviews stripping my comments.

Show 7 quoted lines

> If the patch were to consistently remove "GIT_TRACE=1" placed on
> "git rebase" from all test scripts that do not check the trace
> output consistently (i.e. 3400, 3402, and 7402), with a different
> justification, e.g. "whoever wants to debug a specific part of the
> test can add GIT_TRACE=1 before running the test with -v, but it
> should not be committed", the change would be very reasonable, I
> would think.

Quite frankly, I think -v is completely useless; who likes to scroll through pages of terminal output? If there are really users of this thing, why haven't they got it to auto-pager yet?

I just comment out the test_expect_success and close-quote, and put a test_done after it. I would never advocate this GIT_TRACE thing anywhere, because I want to put GIT_TRACE=1 (and possibly other modifications) where I want it. Locally.