Re: [PATCH v4 1/3] t: do not hide Git's exit code in tests

2016-02-16 Thread Lars Schneider

On 15 Feb 2016, at 18:41, Jeff King  wrote:

> On Mon, Feb 15, 2016 at 11:17:44AM +0100, larsxschnei...@gmail.com wrote:
> 
>> From: Lars Schneider 
>> 
>> Git should not be on the left-hand side of a pipe, because it hides the exit
>> code, and we want to make sure git does not fail.
> 
> I think this is a nice cleanup.
> 
>> There is one more occurrence of this pattern in t9010-svn-fe.sh which is to
>> evolved to change it easily.
> 
> The final sentence in the commit message needs s/to/too/.
OK!

> However, I'm
> not sure this is the only remaining case. Doing:
> 
>  git grep -E 'git.*[^|]\|($|[^|])'
> 
> shows quite a few. I guess you just looked for "nul_to_q". There is
> certainly no need to fix all of them, but you may want to note the
> extent of your grepping in your commit message.
Yes, I looked only for nul_to_q... there are more than 1000 other cases. 
I thought about a clever sed script to fix the easy ones but I don't
think it is worth the effort.

Thanks,
Lars


--
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


Re: [PATCH v4 1/3] t: do not hide Git's exit code in tests

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 11:17:44AM +0100, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> Git should not be on the left-hand side of a pipe, because it hides the exit
> code, and we want to make sure git does not fail.

I think this is a nice cleanup.

> There is one more occurrence of this pattern in t9010-svn-fe.sh which is to
> evolved to change it easily.

The final sentence in the commit message needs s/to/too/. However, I'm
not sure this is the only remaining case. Doing:

  git grep -E 'git.*[^|]\|($|[^|])'

shows quite a few. I guess you just looked for "nul_to_q". There is
certainly no need to fix all of them, but you may want to note the
extent of your grepping in your commit message.

-Peff
--
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


[PATCH v4 1/3] t: do not hide Git's exit code in tests

2016-02-15 Thread larsxschneider
From: Lars Schneider 

Git should not be on the left-hand side of a pipe, because it hides the exit
code, and we want to make sure git does not fail.

There is one more occurrence of this pattern in t9010-svn-fe.sh which is to
evolved to change it easily.

Helped-by: Jeff King 
Signed-off-by: Lars Schneider 
---
 t/t1300-repo-config.sh | 6 --
 t/t7008-grep-binary.sh | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 52678e7..1782add 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -957,13 +957,15 @@ Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF
 test_expect_success '--null --list' '
-   git config --null --list | nul_to_q >result &&
+   git config --null --list >result.raw &&
+   nul_to_q result &&
echo >>result &&
test_cmp expect result
 '
 
 test_expect_success '--null --get-regexp' '
-   git config --null --get-regexp "val[0-9]" | nul_to_q >result &&
+   git config --null --get-regexp "val[0-9]" >result.raw &&
+   nul_to_q result &&
echo >>result &&
test_cmp expect result
 '
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index b146406..9c9c378 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -141,7 +141,8 @@ test_expect_success 'grep respects not-binary diff 
attribute' '
test_cmp expect actual &&
echo "b diff" >.gitattributes &&
echo "b:binQary" >expect &&
-   git grep bin b | nul_to_q >actual &&
+   git grep bin b >actual.raw &&
+   nul_to_q actual &&
test_cmp expect actual
 '
 
-- 
2.5.1

--
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