Re: [PATCH 1/2] Rename submodule.<name>.rebase to submodule.<name>.update

[Johan Herland <johan@herland.net>]

Hi,

Thanks for the feedback. Really appreciated!

On Wednesday 03 June 2009, Markus Heidelberg wrote:
> Johan Herland, 03.06.2009:
> > Therefore, while "submodule.<name>.rebase" are not yet in a stable git
> > release, future-proof it, by changing it from
> >
> >   submodule.<name>.rebase = true/false
> >
> > to
> >
> >   submodule.<name>.update = checkout/rebase
> >
> > where "checkout" specifies the default behaviour of "git submodule
> > update" (checking out the new commit to a detached HEAD), and "rebase"
> > specifies the --rebase behaviour (where the current local branch in the
> > submodule is rebase onto the new commit). Thus .update == checkout is
> > .rebase == false, and .update == rebase is equivalent to .rebase ==
> > false. Finally, leaving
>   ^^^^^
> .rebase == true

Indeed. Thanks.

> > diff --git a/git-submodule.sh b/git-submodule.sh
> > @@ -400,9 +400,9 @@ cmd_update()
> >  			die "Unable to find current revision in submodule path '$path'"
> >  		fi
> >
> > -		if test true = "$rebase"
> > +		if ! test -z "$update"
>
> Isn't this simpler: if test -n "$update"
> OTOH I think I have heard something about portability, but I'm not sure.

Yes, "test -n" is simpler than "! test -z", but at an earlier occasion I was 
told that "test -n" is not as portable as "test -z"...

> > @@ -420,16 +420,18 @@ cmd_update()
> >  				die "Unable to fetch in submodule path '$path'"
> >  			fi
> >
> > -			if test true = "$rebase_module"
> > -			then
> > -				command="git-rebase"
> > +			case "$update_module" in
> > +			rebase)
> > +				command="git rebase"
>
> I think it is common practice to use the dashed form in scripts and this
> patch shouldn't change it anyway.

I thought we were moving away from the dashed form altogether, both in 
scripts and in examples. Junio, is there an "official" position?

> >  				action="rebase"
> >  				msg="rebased onto"
> > -			else
> > -				command="git-checkout $force -q"
> > +				;;
> > +			*)
> > +				command="git checkout $force -q"
>
> ditto

Thanks for your comments, I'll send an updated patch.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net