Nguyễn Thái Ngọc Duy  <> writes:

> Over the time the default value for --thin has been switched between
> on and off. As of now it's always on, even if --no-thin is given.
> Correct the code to respect --no-thin.
> receive-pack learns about --no-thin only for testing purposes, hence
> no document update.

Please name it "--reject-thin-pack-for-testing" to make it crystal
clear that a documentation patch to "document undocumented option"
is unwanted.

> diff --git a/builtin/push.c b/builtin/push.c
> index 04f0eaf..333a1fb 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -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, 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.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to