Subject: Re: [v2 PATCH 2/2] reset: add tests for git reset -

From: Torsten Bögershausen <[email-masked]>
Date: Tue, 10 Mar 2015 18:23:25 +0100

On 2015-03-10 16.38, Sundararajan R wrote:

> Helped-by: Torsten BÃ¶gershausen <[email-masked]>
There seems to be an issue that the mail is encoded
from (what ? Latin-1) into UTF-8 2 times

The easy solution is to remove the line,
I'm OK with that, since a review-comment is not necessarily  motivating
a Helped-by, at least not for me.
Mentioning it in the comments good and is enough.

But why is the mail "encoded twice" ? (this what the header says:)
  X-Mailer: git-send-email 2.1.0
  Content-Type: text/plain; charset=UTF-8

Can somebody help out with a good explanation ?

Another (minor) thing:
There is nothing wrong with the test, but we can make it 3% more "Git-style" and
easier too read when it is more similar to the rest of the code base:

test_expect_success 'reset - with @{-1} and no file named - or @{-1} should succeed' '
+	git init new &&
+	(
+		cd new &&
+		echo "Hey" >new_file &&
+		git add new_file &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>new_file &&
+		git add new_file &&
+		git reset - &&
+		git status -uno >file1 &&
(Side-question: why "status -uno")
typically "file" (or "file1") is used for user files, not for the "expected" or "actual" output.

Then we can compare the files directly in new/.
And if we use new1, new2, new3, we don't need the explicit cleanup, as all tests
are run in a "trash directory" which will be removed anyway.

In other words, we can write like this:
(But this is for discussion, please read it as a suggestion)

+test_expect_success 'reset - with @{-1} and no file named - or @{-1} should succeed' '
+	git init new3 &&
+	(
+		cd new3 &&
+		echo "Hey" >new_file &&
+		git add new_file &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>new_file &&
+		git add new_file &&
+		git reset - &&
+		git status -uno >expected &&
+		git add new_file &&
+		git reset @{-1} &&
+		git status -uno >actual
+		test_cmp expected actual 
+	)
+'