Ramkumar Ramachandra <artag...@gmail.com> writes:
> diff --git a/t/t4041-diff-submodule-option.sh
> b/t/t4041-diff-submodule-option.sh
> index 6c01d0c..e401814 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -33,6 +33,7 @@ test_create_repo sm1 &&
> add_file . foo >/dev/null
>
> head1=$(add_file sm1 foo1 foo2)
> +fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
That looks like quite a roundabout way to say
$(cd sm1; git rev-parse --verify HEAD)
doesn't it? I know this is just moved code from the original, so I
am not saying this should be fixed in this commit.
Existing code in t7401 may want to be cleaned up, perhaps after this
topic settles. The add_file() function, for example, has at least
these points:
- do we know 7 hexdigits is always the right precision?
- what happens when it fails to create a commit in one of the steps
while looping over "$@" in its loop?
- the function uses the "cd there and then come back", not
"go there in a subshell and do whatever it needs to" pattern.
> +test_expect_success 'added submodule, set diff.submodule' "
s/added/add/;
Shouldn't it test the base case where no diff.submodule is set? For
those people, the patch should not change the output when they do or
do not pass --submodule option, right?
> + git config diff.submodule log &&
> + git add sm1 &&
> + git diff --cached >actual &&
> + cat >expected <<-EOF &&
> +Submodule sm1 0000000...$head1 (new submodule)
> +EOF
> + git config --unset diff.submodule &&
> + test_cmp expected actual
> +"
> +
> +test_expect_success '--submodule=short overrides diff.submodule' "
> + git config diff.submodule log &&
> + git add sm1 &&
> + git diff --submodule=short --cached >actual &&
> + cat >expected <<-EOF &&
> +diff --git a/sm1 b/sm1
> +new file mode 160000
> +index 0000000..a2c4dab
> +--- /dev/null
> ++++ b/sm1
> +@@ -0,0 +1 @@
> ++Subproject commit $fullhead1
> +EOF
> + git config --unset diff.submodule &&
> + test_cmp expected actual
> +"
> +
> commit_file sm1 &&
> head2=$(add_file sm1 foo3)
>
> @@ -73,7 +102,6 @@ EOF
> test_cmp expected actual
> "
>
> -fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
> fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
> test_expect_success 'modified submodule(forward) --submodule=short' "
> git diff --submodule=short >actual &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
