Caleb Thompson [off-list ref] writes:
On Fri, Jun 13, 2014 at 10:48:55AM -0700, Junio C Hamano wrote:
quoted
Caleb Thompson [off-list ref] writes:
quoted
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 35a4d06..402d6a1 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -7,6 +7,10 @@ write_script check-for-diff <<-'EOF'
exec grep '^diff --git' "$1"
EOF
+write_script check-for-no-diff <<-'EOF'
+ exec grep -v '^diff --git' "$1"
+EOF
This lets grep show all lines that are not "diff --git" in the
input, and as usual grep exits success if it has any line in the
output.
$ grep -v '^diff --git' <<\EOF ; echo $?
diff --git
a
EOF
a
0
$ exit
What are we testing, exactly?
Good catch. It worked when I switched check-for-diff from
check-for-no-diff, but I didn't try to make check-for-no-diff fail
independently, so I apologize.
No need to apologize at all. None of us (including this reviewer)
is perfect and that is why we review patches by each other.
This version removes the the beginning of a line starting with
"diff --git" from the string,...
Again, what are we testing, exactly?
We do not want to see "^diff --git" in the output file, in other
words, we want to make sure "^diff --git" does not appear in the
output.
So
write_script check-for-no-diff <<-\EOF
! grep '^diff --git' "$@"
EOF
should be the most natural way to express what we are testing, no?
On Mon, Jun 16, 2014 at 01:06:45PM -0700, Junio C Hamano wrote:
Caleb Thompson [off-list ref] writes:
quoted
On Fri, Jun 13, 2014 at 10:48:55AM -0700, Junio C Hamano wrote:
quoted
Caleb Thompson [off-list ref] writes:
quoted
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 35a4d06..402d6a1 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -7,6 +7,10 @@ write_script check-for-diff <<-'EOF'
exec grep '^diff --git' "$1"
EOF
+write_script check-for-no-diff <<-'EOF'
+ exec grep -v '^diff --git' "$1"
+EOF
This lets grep show all lines that are not "diff --git" in the
input, and as usual grep exits success if it has any line in the
output.
$ grep -v '^diff --git' <<\EOF ; echo $?
diff --git
a
EOF
a
0
$ exit
What are we testing, exactly?
Good catch. It worked when I switched check-for-diff from
check-for-no-diff, but I didn't try to make check-for-no-diff fail
independently, so I apologize.
No need to apologize at all. None of us (including this reviewer)
is perfect and that is why we review patches by each other.
quoted
This version removes the the beginning of a line starting with
"diff --git" from the string,...
Again, what are we testing, exactly?
We do not want to see "^diff --git" in the output file, in other
words, we want to make sure "^diff --git" does not appear in the
output.
So
write_script check-for-no-diff <<-\EOF
! grep '^diff --git' "$@"
EOF
should be the most natural way to express what we are testing, no?
I did consider that. The reason I didn't propose that is that it doesn't catch
the unlikely case that the $1 only contains a "diff --git" line or that $1 is
empty.
Those are rather unreasonable concerns, so I'm happy to use the much more
readable version as you propose.
Caleb Thompson