Re: [PATCH v2 1/8] t4051: rewrite, add more tests
flat view
From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:38
Possibly related (same subject, not in this thread)
- 2016-06-16 · [PATCH v2 1/8] t4051: rewrite, add more tests · René Scharfe <hidden>
René Scharfe [off-list ref] writes:
quoted hunk
+commit_and_tag () { + message=$1 && + shift && + git add $@ &&
Lack of dq around $@ makes me wonder if there is something funny going on (looking at the callers, there isn't, so we'd better quote it to avoid wasting time, I think).
quoted hunk
+ test_tick && + git commit -m $message && + git tag $message }
The use of $message as the sole argument to "git tag" makes the readers guess that it must be a single token without any funny character, so the readers would probably do not waste too much time wondreing if the lack of dq around $message in the last two is problematic.
quoted hunk
+last_context_line () { + sed -n '$ p' }
I have a vague recollection that some implementations of sed are unhappy to see that space between the address and the operation; I'd feel safer without it.
quoted hunk
+check_diff () { + name=$1 + desc=$2 + options="-W $3" + + test_expect_success "$desc" ' + git diff $options "$name^" "$name" >"$name.diff" + ' + + test_expect_success ' diff applies' ' + test_when_finished "git reset --hard" && + git checkout --detach "$name^" &&
With the presence of ^ there, --detach is unnecessary; it would not hurt, though.
+ git apply "$name.diff" && + git diff --exit-code "$name"
Even though we may know that $name.diff" will never have a creation of new paths, I'd feel safer if "apply" is run with "--index". Thanks.