From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 28 Jul 2011 20:05:45 GMT
Subject: Re: [PATCH v2 3/3] push: Add the --no-recurse-submodules option
Message-ID: <7vy5zikw6e.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1311790250-32454-4-git-send-email-iveqy@iveqy.com>

Fredrik Gustafsson <iveqy@iveqy.com> writes:

> This adds the option --no-recurse-submodules to push. That is, git

I think this needs to be renamed at least for two reasons.

The name makes it sound as if "push --recurse-submodules" would
recursively visit the submodules and runs "push" there, but I do not think
that is what this flag does.

> diff --git a/transport.h b/transport.h
> index 161d724..c6ccf8c 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -101,6 +101,7 @@ struct transport {
>  #define TRANSPORT_PUSH_MIRROR 8
>  #define TRANSPORT_PUSH_PORCELAIN 16
>  #define TRANSPORT_PUSH_SET_UPSTREAM 32
> +#define TRANSPORT_PUSH_NO_RECURSE_SUBMODULES 64

Also naming the constant with NO will invite an unnecessary double
negation, like this:

	if (!(flags & FROTZ_NO_NITFOL))
        	do_nitfol_to_frotz();

Besides, I would be moderately annoyed if this check were the default.