"W. Trevor King" <wk...@tremily.us> writes:

> This avoids the current awkwardness of having either '' or 'checkout'
> for checkout-mode updates, which makes testing for checkout-mode
> updates (or non-checkout-mode updates) easier.
>
> Signed-off-by: W. Trevor King <wk...@tremily.us>
> ---
>  git-submodule.sh | 27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 5247f78..5e8776c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -803,17 +803,10 @@ cmd_update()
>                       update_module=$update
>               else
>                       update_module=$(git config submodule."$name".update)
> -                     case "$update_module" in
> -                     '')
> -                             ;; # Unset update mode
> -                     checkout | rebase | merge | none)
> -                             ;; # Known update modes
> -                     !*)
> -                             ;; # Custom update command
> -                     *)
> -                             die "$(eval_gettext "Invalid update mode 
> '$update_module' for submodule '$name'")"
> -                             ;;
> -                     esac
> +                     if test -z "$update_module"
> +                     then
> +                             update_module="checkout"
> +                     fi
>               fi

Is this a good change?

It removes the code to prevent a broken configuration value from
slipping through.  The code used to stop early to give the user a
chance to fix it before actually letting "submodule update" to go
into the time consuming part, e.g. a call to module_clone, but that
code is now lost.

>               displaypath=$(relative_path "$prefix$sm_path")
> @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?")"
>                       case ";$cloned_modules;" in
>                       *";$name;"*)
>                               # then there is no local change to integrate
> -                             update_module= ;;
> +                             update_module=checkout ;;
>                       esac
>  
>                       must_die_on_failure=
>                       case "$update_module" in
> +                     checkout)
> +                             command="git checkout $subforce -q"
> +                             die_msg="$(eval_gettext "Unable to checkout 
> '\$sha1' in submodule path '\$displaypath'")"
> +                             say_msg="$(eval_gettext "Submodule path 
> '\$displaypath': checked out '\$sha1'")"
> +                             ;;
>  ...
>                       *)
> -                             command="git checkout $subforce -q"
> -                             die_msg="$(eval_gettext "Unable to checkout 
> '\$sha1' in submodule path '\$displaypath'")"
> -                             say_msg="$(eval_gettext "Submodule path 
> '\$displaypath': checked out '\$sha1'")"
> -                             ;;
> +                             die "$(eval_gettext "Invalid update mode 
> '$update_module' for submodule '$name'")"
>                       esac

These two hunks make sense.  It is clear in the updated code that
"checkout" is what is implemented with that "git checkout $subforce
-q", and not any other random update mode.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to