From: Junio C Hamano <gitster@pobox.com>
To: AbdAlRahman Gad <abdobngad@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 0/8] [Newcomer] t7004: modernize the style
Date: Thu, 08 Aug 2024 08:42:43 -0700 [thread overview]
Message-ID: <xmqq5xsbkolo.fsf@gitster.g> (raw)
In-Reply-To: <20240807130259.28381-1-abdobngad@gmail.com> (AbdAlRahman Gad's message of "Wed, 7 Aug 2024 15:58:36 +0300")
AbdAlRahman Gad <abdobngad@gmail.com> writes:
> - Remove whitespace after the redirect operators.
>
> - Move number of expect files prepared outside of
> test_expect_success to be inside the tests that use it.
>
> - Split some lines that have two commands into two lines
> one command each.
>
> - Turn some "<<\EOF" into "<<-\EOF" and indent their body.
>
> - Avoid using pipes in the output from "test-tool ref-store"
> and write the output to a file.
>
> - Change test_expect_success that are seperated from its name
> to be on the same line.
>
> - Avoid separating test Description and test body with backslash
>
> - Use single quotes instead of double quotes for test description and
> body.
>
> - Use write_script which takes care of emitting the `#!/bin/sh` line
> and the `chmod +x`.
I gave another look and they all looked fine. Except the title of
one step that said
t7004: test Description and test body seperated with backslash
was a "Huh? what does it even mean?".
t7004: begin the test body on the same line as test_expect_success
or something? I dunno.
> There are still tests that could lose exit status to pipe. This needs
> to be modernized too, I will fix them in another patch series.
;-)
Another one that I noticed is that we have quite a lot of
cat >expect <<-EOF &&
v1.1.3
v2.0
v3.0
EOF
that can be shortend to
test_write_lines >expect v1.1.3 v2.0 v3.0 &&
To use without extra quoting, test_write_lines is more limited, but
the majority of here-doc used for expect files in this test are
enumeration of tag names that we can write without any extra frills,
and test_write_lines may be a very good fit for these use cases.
Thanks.
next prev parent reply other threads:[~2024-08-08 15:42 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-07 12:58 [PATCH v5 0/8] [Newcomer] t7004: modernize the style AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 1/8] t7004: remove space after redirect operators AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 2/8] t7004: one command per line AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 3/8] t7004: do not prepare things outside test_expect_success AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 4/8] t7004: use indented here-doc AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 5/8] t7004: description on the same line as test_expect_success AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 6/8] t7004: test description and test body seperated with backslash AbdAlRahman Gad
2024-08-07 12:58 ` [PATCH v5 7/8] t7004: use single quotes instead of double quotes AbdAlRahman Gad
2024-08-08 15:36 ` Junio C Hamano
2024-08-07 12:58 ` [PATCH v5 8/8] t7004: make use of write_script AbdAlRahman Gad
2024-08-08 15:42 ` Junio C Hamano [this message]
2024-08-08 16:18 ` [PATCH v5 0/8] [Newcomer] t7004: modernize the style 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=xmqq5xsbkolo.fsf@gitster.g \
--to=gitster@pobox.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 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.