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
+ )
+'
next prev parent 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).