From: Junio C Hamano <gitster@pobox.com>
To: "Amanda Shafack via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>,
Jonathan Nieder <jrnieder@gmail.com>,
Eric Sunshine <sunshine@sunshineco.com>,
Amanda Shafack <shafack.likhene@gmail.com>
Subject: Re: [PATCH v3] t2200,t9832: avoid using 'git' upstream in a pipe
Date: Sun, 18 Oct 2020 14:01:54 -0700 [thread overview]
Message-ID: <xmqqv9f7uu2l.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <pull.885.v3.git.git.1603054088108.gitgitgadget@gmail.com> (Amanda Shafack via GitGitGadget's message of "Sun, 18 Oct 2020 20:48:07 +0000")
"Amanda Shafack via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Amanda Shafack <shafack.likhene@gmail.com>
>
> Avoid placing `git` upstream in a pipe since doing so throws away
> its exit code, thus an unexpected failure may go unnoticed.
Well explained.
> Signed-off-by: Amanda Shafack <shafack.likhene@gmail.com>
> ---
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index f764b7e3f5..7cb7a70382 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -179,7 +179,8 @@ test_expect_success 'add -u resolves unmerged paths' '
>
> test_expect_success '"add -u non-existent" should fail' '
> test_must_fail git add -u non-existent &&
> - ! (git ls-files | grep "non-existent")
> + git ls-files >actual &&
> + ! grep "non-existent" actual
> '
In the scope of this patch, the above change is a good rewrite.
Let's stop the iteration here---you've demonstrated through the
microproject that you can now comfortably be involved in the review
discussion.
In a larger scope of "can we write this particular line better?",
however, the above may not be the _best_ answer. For example,
test_must_fail git ls-files --error-unmatch non-existent
would be another and a more direct way to ensure that the named path
does not appear in the index.
Thanks.
prev parent reply other threads:[~2020-10-18 21:02 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-17 16:02 [PATCH 0/2] [Outreachy][Patch v1] t9832,t2200: avoid using pipes in git related commands Amanda Shafack via GitGitGadget
2020-10-17 16:02 ` [PATCH 1/2] t9832,t2200: avoid using pipes in git commands Amanda Shafack via GitGitGadget
2020-10-18 6:11 ` Eric Sunshine
2020-10-18 13:57 ` Amanda Shafack
2020-10-17 16:02 ` [PATCH 2/2] t2200: modify code syntax Amanda Shafack via GitGitGadget
2020-10-18 5:55 ` Eric Sunshine
2020-10-18 13:59 ` Amanda Shafack
2020-10-18 14:42 ` [PATCH v2] t9832,t2200: avoid using pipes in git commands Amanda Shafack via GitGitGadget
2020-10-18 19:25 ` Eric Sunshine
2020-10-18 20:04 ` Junio C Hamano
2020-10-18 22:04 ` Amanda Shafack
2020-10-18 20:40 ` Amanda Shafack
2020-10-18 20:48 ` [PATCH v3] t2200,t9832: avoid using 'git' upstream in a pipe Amanda Shafack via GitGitGadget
2020-10-18 21:01 ` Junio C Hamano [this message]
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=xmqqv9f7uu2l.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=jrnieder@gmail.com \
--cc=shafack.likhene@gmail.com \
--cc=sunshine@sunshineco.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.