From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: "Siddharth Asthana" <siddharthasthana31@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Derrick Stolee" <derrickstolee@github.com>,
git <git@vger.kernel.org>,
"Lars Schneider" <larsxschneider@gmail.com>,
"Elijah Newren" <newren@gmail.com>
Subject: Re: [GSoC] [PATCH v2] t1011: replace test -f with test_path_is_file
Date: Thu, 14 Apr 2022 09:42:34 -0700 [thread overview]
Message-ID: <xmqqy2071urp.fsf@gitster.g> (raw)
In-Reply-To: <CAP8UFD3RtQecxOQWVeapH1CYsMiY2+MoHeugb9bvOsFVnozy=w@mail.gmail.com> (Christian Couder's message of "Thu, 14 Apr 2022 10:19:07 +0200")
Christian Couder <christian.couder@gmail.com> writes:
> In the commit message it might be nice if there were some explanations
> about why `test_path_is_missing PATH` should be used instead of `!
> test_path_is_file PATH` or `test ! -f PATH` or `! test -f PATH`.
Meaning "the original used '! test -f foo' but what it meant was
that it did not want to see 'foo' on the filesystem, regardless of
its type, so it should have been '!test -e foo' to begin with"?
I guess it does not hurt, as the original would have passed by
mistake if these paths were on the filesystem as directories, but
the new code would behave differently, and even if it is a "bugfix",
it still is a behaviour change that may be worth explaining.
> The diff part of the patch looks good to me. Thanks!
next prev parent reply other threads:[~2022-04-14 17:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-09 11:44 [GSoC] [PATCH] t1011: replace test -f with test_path_is_file Siddharth Asthana
2022-04-11 19:09 ` Junio C Hamano
2022-04-12 20:21 ` Siddharth Asthana
2022-04-12 20:37 ` [GSoC] [PATCH v2] " Siddharth Asthana
2022-04-14 8:19 ` Christian Couder
2022-04-14 16:42 ` Junio C Hamano [this message]
2022-04-16 13:55 ` Siddharth Asthana
2022-04-16 13:59 ` [PATCH v3] t1011: replace test -f with test_path_is* helpers Siddharth Asthana
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=xmqqy2071urp.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=christian.couder@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=larsxschneider@gmail.com \
--cc=newren@gmail.com \
--cc=siddharthasthana31@gmail.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 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).