From: Shawn O. Pearce <spearce@spearce.org>
Date: Thu, 25 Sep 2008 14:54:07 GMT
Subject: Re: [PATCH] git-submodule: remove unnecessary exits when calling resolve_relative_url
Message-ID: <20080925145407.GM3669@spearce.org>
In-Reply-To: <48DB81DE.6070600@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> wrote:
> David Aguilar schrieb:
> > resolve_relative_url calls die() when no remote url exists so these calls to
> > exit can be removed.
> ...
> > @@ -155,7 +155,7 @@ cmd_add()
> >  	case "$repo" in
> >  	./*|../*)
> >  		# dereference source url relative to parent's url
> > -		realrepo=$(resolve_relative_url "$repo") || exit
> > +		realrepo=$(resolve_relative_url "$repo")
> >  		;;
> 
> Did you test it? The command inside $(...) is run in its own sub-process,
> therefore, the die() does not exit the caller, just the sub-process, and
> the || exit *is* required.
> 
> BTW, I think that || exit is sufficient; you don't need to add another
> error message - the one that resolve_relative_url() prints is sufficient.

Exactly.

I think we just need a "|| exit" after each of these
$(resolve_relative_url) calls.  The original patch that
started this discussion just needs a "|| exit".

die with an additional message is just too verbose.


-- 
Shawn.