Re: [PATCH v3 5/7] submodule: use the 'submodule--helper config' command

From: Brandon Williams <bmwill@google.com>
Date: Aug 14, 2018, 17:12 UTC
Message-ID: <20180814171241.GA233973@google.com>
In-Reply-To: <20180814110525.17801-6-ao2@ao2.it>

On 08/14, Antonio Ospite wrote:

Show 7 quoted lines

> Use the 'submodule--helper config' command in git-submodules.sh to avoid
> referring explicitly to .gitmodules by the hardcoded file path.
> 
> This makes it possible to access the submodules configuration in a more
> controlled way.
> 
> Signed-off-by: Antonio Ospite <ao2@ao2.it>

Looks great. I also like you're approach of introducing the new API and testing it in one commit, and then using it in the next. Makes the patch set very easy to follow.

Show 35 quoted lines

> ---
>  git-submodule.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8b5ad59bde..ff258e2e8c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -72,7 +72,7 @@ get_submodule_config () {
>  	value=$(git config submodule."$name"."$option")
>  	if test -z "$value"
>  	then
> -		value=$(git config -f .gitmodules submodule."$name"."$option")
> +		value=$(git submodule--helper config submodule."$name"."$option")
>  	fi
>  	printf '%s' "${value:-$default}"
>  }
> @@ -283,11 +283,11 @@ or you are unsure what this means choose another name with the '--name' option."
>  	git add --no-warn-embedded-repo $force "$sm_path" ||
>  	die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
>  
> -	git config -f .gitmodules submodule."$sm_name".path "$sm_path" &&
> -	git config -f .gitmodules submodule."$sm_name".url "$repo" &&
> +	git submodule--helper config submodule."$sm_name".path "$sm_path" &&
> +	git submodule--helper config submodule."$sm_name".url "$repo" &&
>  	if test -n "$branch"
>  	then
> -		git config -f .gitmodules submodule."$sm_name".branch "$branch"
> +		git submodule--helper config submodule."$sm_name".branch "$branch"
>  	fi &&
>  	git add --force .gitmodules ||
>  	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
> -- 
> 2.18.0
>

-- 
Brandon Williams