On Mon, Aug 12, 2013 at 12:59 PM, Junio C Hamano <gits...@pobox.com> wrote:
>> @@ -15,7 +15,7 @@ static const char * const push_usage[] = {
>>       NULL,
>>  };
>>
>> -static int thin;
>> +static int thin = 1;
>>  static int deleterefs;
>>  static const char *receivepack;
>>  static int verbosity;
>> @@ -313,8 +313,7 @@ static int push_with_options(struct transport 
>> *transport, int flags)
>>       if (receivepack)
>>               transport_set_option(transport,
>>                                    TRANS_OPT_RECEIVEPACK, receivepack);
>> -     if (thin)
>> -             transport_set_option(transport, TRANS_OPT_THIN, "yes");
>> +     transport_set_option(transport, TRANS_OPT_THIN, thin ? "yes" : NULL);
>>
>>       if (verbosity > 0)
>>               fprintf(stderr, _("Pushing to %s\n"), transport->url);
>
> Hmm, I am utterly confused.
>
> How does the original code have thin as an non-overridable default?
> The variable is initialized to 0, parse-options specifies it as
> OPT_BOOLEAN, and TRANS_OPT_THIN is set only if "thin" is set.
>
> Updated code flips the default to "1" but unconditionally uses
> TRANS_OPT_THIN to set it to either "yes" or NULL.  While it is not
> wrong per-se, do_push() (i.e. the caller of this function) grabs the
> transport from transport_get() which initializes the transport with
> the thin option disabled by default,

transport_get() actually sets thin option to 1 by default. If I don't
misread the code, the first version of transport.c already flipped
"thin" from 0 (in push.c) to 1 (in transport.c), see 9b28851 (Push
code for transport library - 2007-09-10). The funny thing is that
commit was just one day after Shawn flipped 'thin' from 1 to 0 in
push.c in a4503a1.

> so it is not immediately
> obvious to me why "always call TRANS_OPT_THIN but set it explicitly
> to NULL when --no-thin is asked" is necessary or improvement.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to