On Sun, Mar 17, 2019 at 03:15:50AM -0700, Denton Liu wrote:
> Sorry for taking so long to do a reroll, I've been pretty busy this week
> and I've only been able to find the time now.
No problem, and thank you for sticking with it!
> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
> index ca4a740da0..f035e4a507 100755
> --- a/t/t7502-commit-porcelain.sh
> +++ b/t/t7502-commit-porcelain.sh
> @@ -16,7 +16,9 @@ commit_msg_is () {
> # Arguments: [<prefix] [<commit message>] [<commit options>]
> check_summary_oneline() {
> test_tick &&
> - git commit ${3+"$3"} -m "$2" | head -1 > act &&
> + git commit ${3+"$3"} -m "$2" >act &&
> + head -1 <act >tmp &&
> + mv tmp act &&
Here you could swap the order of when you write to 'act' and 'tmp',
and thus make the 'mv' unnecessary, like this:
git commit [...] >tmp &&
head -1 act >tmp &&
[...rest of the test...]
Note also that 'head' (or 'sed' in later tests) can open its input
file on its own, there's no need to redirect its standard input.
This is a recurring pattern in patches 1, 4, 8, and 9.
> @@ -142,8 +144,8 @@ test_expect_success 'sign off' '
> >positive &&
> git add positive &&
> git commit -s -m "thank you" &&
> - actual=$(git cat-file commit HEAD | sed -ne "s/Signed-off-by: //p") &&
> - expected=$(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/") &&
> + actual=$(git cat-file commit HEAD >tmp && sed -ne "s/Signed-off-by:
> //p" <tmp && rm tmp) &&
> + expected=$(git var GIT_COMMITTER_IDENT >tmp && sed -e "s/>.*/>/" <tmp
> && rm tmp) &&
> test "z$actual" = "z$expected"
May I ask you to go one step further in restructuring this and the
following tests? :) Instead of using 'test' to compare the contents
of the $actual and $expected variables, use 'test_cmp' to compare the
'actual' and 'expected' files, something like:
git cat-file commit HEAD >tmp &&
sed -ne "s/Signed-off-by: //p" tmp >actual &&
git var GIT_COMMITTER_IDENT >tmp &&
sed -e "s/>.*/>/" >expected &&
test_cmp expected actual
