Re: [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit

From: Francesco Pretto <ceztko@gmail.com>
Date: Jan 16, 2014, 20:07 UTC
Message-ID: <CALas-ig4=qKqpopaP163gVdBG39seWH61qqA5Jx0CR6AZkE6ZA@mail.gmail.com>
In-Reply-To: <20140116192252.GT2647@odin.tremily.us>

2014/1/16 W. Trevor King <wking@tremily.us>:

> Avoiding useless clones is probably more important than avoiding
> duplicate "Invalid update mode" messages.

No, it's not duplicate code. I'll explain, please follow me:

Show 15 quoted lines

> @@ -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

This is a *validation*. It's done before going more through the code and die early.

>                       *)
> +                             die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")"
>

This should be an *assert* -> it means if you reach this case statement you (programmer) have messed the code something in the code before. In fact in my original patch I wrote something like "invalid update_module at this flow".

Please keep both as Junio said.

Thanks, Francesco