Re: [PATCHv3 3/4] submodule, repack: migrate to git-sh-setup's say()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2009, 05:56 UTC
Message-ID: <7viqiwg1rl.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1245117905-19351-4-git-send-email-bebarino@gmail.com>

Stephen Boyd <bebarino@gmail.com> writes:

> Now that there is say() in git-sh-setup, these scripts don't need to use
> their own. Migrate them over by setting GIT_QUIET and removing their
> custom say() functions.

This is not exactly a very nice style.

The contract between the callers of say() and its implementation is that it does not matter what value is set to GIT_QUIET. The only thing that matters is if it is set to empty string or not. And ...

Show 6 quoted lines

> @@ -33,7 +33,7 @@ do
>  	-A)	all_into_one=t
>  		unpack_unreachable=--unpack-unreachable ;;
>  	-d)	remove_redundant=t ;;
> -	-q)	quiet=-q ;;
> +	-q)	GIT_QUIET=-q ;;

... this one takes advantage of it to set GIT_QUIET to -q, so that it can be directly passed to another command that happens to use -q as "quiet" option, like this ...

> -	git prune-packed $quiet
> +	git prune-packed $GIT_QUIET
>  fi

... while the other one does not have a callout to command that takes -q at all, and does this ...

>  		-q|--quiet)
> -			quiet=1
> +			GIT_QUIET=1

If the convention is "GIT_QUIET, when set to non-empty string, squelches the output", then I think the callers should be more consistent and the call to prune-packed should say something like this, which is admittedly a roundabout way:

	git prune-packed ${GIT_QUIET:+-q}

for consistency (and then what you set to GIT_QUIET in the first hunk I quoted does not matter anymore---it can even be t or 1 or whatever).

I think this does not matter too much, because I suspect that in the longer term scripted Porcelains are going away, but still...