Re: [PATCH 04/10] test-lib functions: add an --annotated-tag option to "test_commit"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2021, 21:41 UTC
Message-ID: <xmqqr1kyo8l8.fsf@gitster.c.googlers.com>
In-Reply-To: <20210228195414.21372-5-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 7 quoted lines

> @@ -220,6 +225,9 @@ test_commit () {
>  		--no-tag)
>  			no_tag=yes
>  			;;
> +		--annotated-tag)
> +			annotated_tag=yes
> +			;;

A new option is welcome, but shouldn't we be straightening out the variables that keep track of the options around tagging? It's not like it is possible to have 4 (two variables that can independently set to 'yes') possibilities, so something along the lines of ...

         test_commit () {
        +	tag=light &&
                notick= &&
         ...
		while test $# != 0
		do
			case "$1" in
			...
			--no-tag)
	-			no_tag=yes
	+			tag=none
				;;
	+		--annotated)
	+			tag=annotated
	+			;;
		...
	-	if test -z "$no_tag"
	-	then
	+	case "$tag" in
	+	none)
	+		;;
	+	light)
			git ${indir:+ -C "$indir") tag "${4:-$1}"
	+		;;
	+	annotated)
	+		git ${indir:+ -C "$indir") tag -m "$1" "${4:-$1}"
	+		;;
	+	esac
		...

after this step (meaning, we may want to start from fixing the no_tag=yes to tag=none before introducing this new feature).

Thanks.

Show 16 quoted lines

> @@ -244,7 +252,15 @@ test_commit () {
>  	    $signoff -m "$1" &&
>  	if test -z "$no_tag"
>  	then
> -		git ${indir:+ -C "$indir"} tag "${4:-$1}"
> +		if test -n "$annotated_tag"
> +		then
> +			if test -z "$notick"
> +			then
> +				test_tick
> +			fi &&
> +			test_tick
> +		fi &&
> +		git ${indir:+ -C "$indir"} tag ${annotated_tag:+ -a -m "$1"} "${4:-$1}"
>  	fi
>  }