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

From: Johan Herland <johan@herland.net>
Date: Jun 3, 2009, 06:20 UTC
Message-ID: <200906030820.21372.johan@herland.net>
In-Reply-To: <200906030615.45791.markus.heidelberg@web.de>

Hi,

Thanks for the feedback. Really appreciated!

On Wednesday 03 June 2009, Markus Heidelberg wrote:

Show 18 quoted lines

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

Show 10 quoted lines

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

Show 13 quoted lines

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

Show 9 quoted lines

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