Re: [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 16, 2014, 18:46 UTC
- Message-ID
- <xmqqwqhzzrw3.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <43e8f3bfdaffefca9edd7a23574816630690e1e5.1389837412.git.wking@tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
Show 33 quoted lines
> 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 <wking@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.
Show 24 quoted lines
> 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.