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

On Thu, Jan 16, 2014 at 10:46:36AM -0800, Junio C Hamano wrote:

"W. Trevor King" [off-list ref] writes:

quoted

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

Avoiding useless clones is probably more important than avoiding
duplicate "Invalid update mode" messages.  I'll reinstate the case
statement in v5, but I think it should live outside of the “load from
config” block, in case someone adds the ability to set arbitrary
update modes from the command line (`--update merge`, `--update
'!command'`, etc.).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy