On Thu, Jan 16, 2014 at 10:46:36AM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wk...@tremily.us> 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

Reply via email to