git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: Sundararajan R <dyoucme@gmail.com>, git@vger.kernel.org
Subject: Re: [v2 PATCH 2/2] reset: add tests for git reset -
Date: Tue, 10 Mar 2015 18:23:25 +0100	[thread overview]
Message-ID: <54FF288D.3000304@web.de> (raw)
In-Reply-To: <1426001883-6423-2-git-send-email-dyoucme@gmail.com>

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

> Helped-by: Torsten Bögershausen <tboegi@web.de>
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 
+	)
+'

  reply	other threads:[~2015-03-10 17:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-10 15:38 [v2 PATCH 1/2] reset: add '-' shorthand for '@{-1}' Sundararajan R
2015-03-10 15:38 ` [v2 PATCH 2/2] reset: add tests for git reset - Sundararajan R
2015-03-10 17:23   ` Torsten Bögershausen [this message]
2015-03-10 17:35   ` Eric Sunshine
2015-03-10 17:25 ` [v2 PATCH 1/2] reset: add '-' shorthand for '@{-1}' Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54FF288D.3000304@web.de \
    --to=tboegi@web.de \
    --cc=dyoucme@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).