Re: [PATCH v2] git-submodule: Don't die when command fails for one submodule

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 5, 2008, 16:48 UTC
Message-ID: <alpine.LSU.1.00.0803051742290.15786@racer.site>
In-Reply-To: <1204730478-22027-1-git-send-email-pkufranky@gmail.com>

Hi,

On Wed, 5 Mar 2008, Ping Yin wrote:

Show 13 quoted lines

> @@ -221,29 +229,40 @@ cmd_init()
>  	done
>  
>  	git ls-files --stage -- "$@" | grep -e '^160000 ' |
> +	{
> +	total=0
> +	success=0
>  	while read mode sha1 stage path
>  	do
> +		total=$(( $total + 1 ))
>  		# Skip already registered paths
> -		name=$(module_name "$path") || exit
> +		name=$(module_name "$path") || continue

What about this case?  Was the comment misleading?

>  		url=$(git config submodule."$name".url)
> -		test -z "$url" || continue
> +		test -n "$url" && success=$(( $success + 1 )) && continue

Why counting?  Why not just 'status=0 && continue'?

Show 19 quoted lines

>  
> -		git config submodule."$name".url "$url" ||
> -		die "Failed to register url for submodule path '$path'"
> -
> -		say "Submodule '$name' ($url) registered for path '$path'"
> +		if git config submodule."$name".url "$url"
> +		then
> +			say "Submodule '$name' ($url) registered for path '$path'"
> +		else
> +			say "Failed to register url for submodule path '$path'"
> +			continue
> +		fi
> +		success=$(( $success + 1 ))
>  	done
> +	test $success = $total
> +	}
>  }
>  
>  #

Note: I have not even begun to audit if one of your returns should not 
have been a "status=0 && continue" instead.

Show 20 quoted lines

> @@ -358,9 +392,11 @@ cmd_status()
>  	done
>  
>  	git ls-files --stage -- "$@" | grep -e '^160000 ' |
> +	{
> +	exit_status=0
>  	while read mode sha1 stage path
>  	do
> -		name=$(module_name "$path") || exit
> +		! name=$(module_name "$path") && exit_status=1 && continue
>  		url=$(git config submodule."$name".url)
>  		if test -z "$url" || ! test -d "$path"/.git
>  		then
> @@ -380,6 +416,8 @@ cmd_status()
>  			say "+$sha1 $path$revname"
>  		fi
>  	done
> +	exit $exit_status
> +	}
>  }

But here you use the simpler paradigm of setting exit_status? Why that complicated and ugly "total" and "success" counting before?

Ciao, Dscho