Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > On Fri, Jan 17, 2020 at 7:24 AM Philippe Blain via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: >> The subshells used in the setup phase of this test are unnecessary. >> Remove them by using 'git -C' and 'test_commit -C'. > > The subshells may not be necessary, but the code feels cleaner before > this patch is applied since all the added "-C foo/bar" noise hurts > readability. So, I'm "meh" on this patch and wouldn't complain if it > was dropped (though I don't insist upon it). I dunno. Each of these subshells did not do much after going into its subdirectory, so repetition of "-C foo/bar" did not bother me that much. >> Signed-off-by: Philippe Blain <levraiphilippeblain@xxxxxxxxx> >> --- >> diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh >> @@ -6,32 +6,16 @@ test_description='Combination of submodules and multiple worktrees' >> - git commit -m "file1 updated" >> + git -C origin/main commit -m "add sub" && >> + test_commit -C origin/sub "file1-updated" file1 file1updated && >> @@ -49,7 +33,7 @@ test_expect_success 'checkout main' ' >> - grep "file1 updated" out >> + grep "file1-updated" out > > Why this change? Is it because test_commit() mishandles the whitespace > in the commit message? If so, it might deserve mention in the commit > message of this patch. (Even better would be to fix test_commit(), if > that is the case.) FWIW I had the same reaction on that dash in "file1 updated".