On Wed, Jun 01, 2016 at 12:37:47PM -0400, Jeff King wrote:
> On Wed, Jun 01, 2016 at 09:19:06AM -0700, Junio C Hamano wrote:
> > diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
> > index 79bc135..5503ec0 100755
> > --- a/t/t7403-submodule-sync.sh
> > +++ b/t/t7403-submodule-sync.sh
> > @@ -62,13 +62,13 @@ test_expect_success 'change submodule' '
> > '
> >
> > reset_submodule_urls () {
> > - local root
> > - root=$(pwd) &&
> > (
> > + root=$(pwd) &&
> > cd super-clone/submodule &&
> > git config remote.origin.url "$root/submodule"
> > ) &&
> > (
> > + root=$(pwd) &&
> > cd super-clone/submodule/sub-submodule &&
> > git config remote.origin.url "$root/submodule"
>
> Hmm. Isn't $root always just going to be $TRASH_DIRECTORY here? There's
> only one caller, which appears to pass an argument which is ignored (?).
>
> It's probably worth doing the minimal thing now and leaving further
> cleanup for a separate patch, though. Cc-ing John Keeping, the author of
> 091a6eb0feed, which added this code.
I can't shed any light on what this is trying to do; I had a look
through the mailing list and this arrived in the final version of the
series without any comment.
Looking at it now I can't see why this is a separate function (that is
called with a parameter it never uses). I wonder if my original
approach was to call this via test_when_finished from the two tests
following this function definition, but that's pure speculation now.
Junio's change is obviously correct as a minimal fix.
I wonder if it's relevant that the "local root" line isn't &&-chained?
Is it possible that on some shells we ignore an error but everything
still works?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html