On Thu, Jan 16, 2014 at 10:46:36AM -0800, Junio C Hamano wrote: > "W. Trevor King" <wking@xxxxxxxxxx> writes: > > @@ -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
Attachment:
signature.asc
Description: OpenPGP digital signature