From: Eric Sunshine <sunshine@sunshineco.com>
To: AbdAlRahman Gad <abdobngad@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v4 8/8] [Newcomer] t7004: Use single quotes instead of double quotes
Date: Tue, 6 Aug 2024 13:21:37 -0400 [thread overview]
Message-ID: <CAPig+cQPXh-wmNYGSEX_3-C3ydF-O7k22tAx8hB_N55ZYo-aDQ@mail.gmail.com> (raw)
In-Reply-To: <207c4f3c-c444-4405-bed7-34186efa1144@gmail.com>
On Tue, Aug 6, 2024 at 6:10 AM AbdAlRahman Gad <abdobngad@gmail.com> wrote:
> On 8/6/24 06:02, Eric Sunshine wrote:
> > We need to exercise a bit of caution when making this sort of change
> > because a "..." string differs from a '...' string in shell. [...]
> > [...]
> > Having said all that, in this case, you seem to have lucked out and
> > nothing broke by changing the double-quotes to single-quotes around
> > the test bodies.
> >
> > However...
> > ... changing the double-quotes to single-quotes for the test _titles_
> > in these instances is actively wrong. In this case, we _want_
> > interpolation of `$option` to happen in the title string so that the
> > output looks like this:
> > ok 167 - mixing incompatible modes with --contains is forbidden
> >
> > By changing the title to use single-quotes, you suppress interpolation
> > of `$option`, with the result that the displayed titles become rather
> > useless:
> > ok 167 - mixing incompatible modes with $option is forbidden
>
> Thanks for the through explanation! Should I not include this patch in
> v5 as the test body is error-prune or should I revert changing the
> double-quotes to single-quotes for test description and keep the changes
> to the test body and send the patch with v5?
Overall, the changes made by this patch are valuable since it brings
the script more in line with modern practice and because single-quotes
around the test body make it easier for readers to reason about test
behavior. Coupled with the fact that none of the tests broke by
changing the bodies from double- to single-quote, it's a good idea to
keep this patch in v5 but omit the parts which break the test
descriptions.
next prev parent reply other threads:[~2024-08-06 17:21 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-05 23:59 [PATCH v4 0/8] t7004: Modernize the style AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 1/8] [Newcomer] t7004: Remove space after redirect operators AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 2/8] [Newcomer] t7004: Do not lose exit status to pipe AbdAlRahman Gad
2024-08-06 3:13 ` Eric Sunshine
2024-08-06 8:38 ` AbdAlRahman Gad
2024-08-06 18:05 ` Eric Sunshine
2024-08-05 23:59 ` [PATCH v4 3/8] [Newcomer] t7004: One command per line AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 4/8] [Newcomer] t7004: Do not prepare things outside test_expect_success AbdAlRahman Gad
2024-08-06 3:29 ` Eric Sunshine
2024-08-06 8:41 ` AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 5/8] [Newcomer] t7004: Use indented here-doc AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 6/8] [Newcomer] t7004: Description on the same line as test_expect_success AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 7/8] [Newcomer] t7004: test Description and test body seperated with backslash AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 8/8] [Newcomer] t7004: Use single quotes instead of double quotes AbdAlRahman Gad
2024-08-06 3:02 ` Eric Sunshine
2024-08-06 10:10 ` AbdAlRahman Gad
2024-08-06 17:21 ` Eric Sunshine [this message]
2024-08-06 16:35 ` Junio C Hamano
2024-08-06 17:11 ` Eric Sunshine
2024-08-06 0:45 ` [PATCH v4 0/8] t7004: Modernize the style Junio C Hamano
2024-08-06 8:44 ` AbdAlRahman Gad
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=CAPig+cQPXh-wmNYGSEX_3-C3ydF-O7k22tAx8hB_N55ZYo-aDQ@mail.gmail.com \
--to=sunshine@sunshineco.com \
--cc=abdobngad@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).