Re: [PATCH 1/2] imap-send.c: implements the GIT_CURL_DEBUG environment variable

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 4, 2016, 16:50 UTC
Message-ID: <xmqqr3el5nen.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CA+EOSBkY-Tsz3ZAfK3uAXJsrE585cOhyxjsU4FhgDMFC-ypkUg@mail.gmail.com>

Elia Pinto <gitter.spiros@gmail.com> writes:

Show 7 quoted lines

>> My impression is that using GIT_TRACE_* is the more mainstream
>> trend, and it may be beneficial to work any new debugging aid like
>> this one to fit within that mechanism.
>
> I thought about it, and I agree with you. The idea could be
>
> - Call the variable GIT_TRACE_CURL_DEBUG instead

I think GIT_TRACE_CURL_DEBUG is overly verbose; tracing by definition is a debugging aid.

> - Add the new GIT_TRACE_CURL_VERBOSE variable, keeping
> GIT_CURL_VERBOSE for compatibility

I do not care too deeply either way.

 - GIT_CURL_VERBOSE can stay the same as-is and show its output to
   whatever output channel it spits things out.

 - Or it can be a synonym for GIT_TRACE_CURL=2 (as I understand that
   the VERBOSE output goes to the standard error stream)

If you want tracing as debugging aid and existing CURL_VERBOSE orthogonal, it would probably make more sense to go the former route, not linking this new "DEBUG" thing with the existing "VERBOSE" thing.

> - Documenting these GIT_TRACE_CURL_XXX variables (GIT_CURL_VERBOSE
> it is not even documented i think)

If we decide to leave them untangled, this is not necessary.

> - perhaps use the git trace api in doing these new patches
>
> Look reasonable? It seems reasonable? I'd like your own opinion

Not really sensible as long as you have that "perhaps" in the list.

Something that does not use the trace API shouldn't pretend to by using GIT_TRACE_* names.

GIT_TRACE_CURL could be your new thing and would decide where to show its output by using the GIT_TRACE_* api.