Re: [PATCH 1/2] push: --dry-run updates submodules when --recurse-submodules=on-demand

[Brandon Williams <bmwill@google.com>]

On 11/15, Johannes Sixt wrote:
> Am 15.11.2016 um 02:18 schrieb Brandon Williams:
> >diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> >index 198ce84..e6ccc30 100755
> >--- a/t/t5531-deep-submodule-push.sh
> >+++ b/t/t5531-deep-submodule-push.sh
> >@@ -427,7 +427,31 @@ test_expect_success 'push unpushable submodule recursively fails' '
> > 		cd submodule.git &&
> > 		git rev-parse master >../actual
> > 	) &&
> >-	test_cmp expected actual
> >+	test_cmp expected actual &&
> >+	git -C work reset --hard master^
> 
> This line looks like a clean-up to be done after the test case. You
> should wrap it in test_when_finished, but outside of a sub-shell,
> which looks like it's just one line earlier, before the test_cmp.

K will do.

> 
> >+'
> >+
> >+test_expect_failure 'push --dry-run does not recursively update submodules' '
> >+	(
> >+		cd work &&
> >+		(
> >+			cd gar/bage &&
> >+			git checkout master &&
> >+			git rev-parse master >../../../expected_submodule &&
> >+			> junk9 &&
> >+			git add junk9 &&
> >+			git commit -m "Ninth junk"
> >+		) &&
> 
> Could you please avoid this nested sub-shell? It is fine to cd
> around when you are in a sub-shell.

Yes I can reorganize it to avoid the nested sub-shells.  I was just
trying to follow the organization of the other tests in the same file.

> 
> >+		git checkout master &&
> >+		git rev-parse master >../expected_pub
> 
> Broken && chain.
> 
> >+		git add gar/bage &&
> >+		git commit -m "Ninth commit for gar/bage" &&
> >+		git push --dry-run --recurse-submodules=on-demand ../pub.git master
> >+	) &&
> >+	git -C submodule.git rev-parse master >actual_submodule &&
> >+	git -C pub.git rev-parse master >actual_pub &&
> 
> All of the commands above are 'git something' that could become 'git
> -C work something' and then the sub-shell would be unnecessary. I'm
> not sure I would appreciate the verbosity of the result, though.
> (Perhaps aligning the git subcommands after -C foo would help.)

I'll play around with it and try to make it look pretty while trying to
avoid sub-shells.  I'm assuming the reason we want to avoid sub-shells is
for performance reasons right?

-- 
Brandon Williams