[v6,0/8,Newcomer] t7004: modernize the style

Message

AbdAlRahman Gad Aug. 8, 2024, 4:31 p.m. UTC

- 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`.

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.

AbdAlRahman Gad (8):
  t7004: remove space after redirect operators
  t7004: one command per line
  t7004: do not prepare things outside test_expect_success
  t7004: use indented here-doc
  t7004: description on the same line as test_expect_success
  t7004: begin the test body on the same line as test_expect_success
  t7004: use single quotes instead of double quotes
  t7004: make use of write_script

 t/t7004-tag.sh | 1144 +++++++++++++++++++++++-------------------------
 1 file changed, 549 insertions(+), 595 deletions(-)


base-commit: 406f326d271e0bacecdb00425422c5fa3f314930

Comments

Eric Sunshine Aug. 13, 2024, 7:57 p.m. UTC | #1

On Thu, Aug 8, 2024 at 12:34 PM AbdAlRahman Gad <[e-mail]> wrote:
> - 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.
> [...]
> 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.

Thanks. This sort of information -- explaining the aim of the series
and explaining what will be done later -- is appreciated by reviewers.

There are a few additional pieces of information you can include in
the cover letter to make life even simpler for reviewers:

* provide a link to the previous version of the series

* explain what differs in this version as compared with the previous version

* include a range-diff showing the actual changes between this version
and the previous version (see `git format-patch --range-diff=`

There are many good examples on the mailing which illustrate the above
points; here is one such instance:
https://lore.kernel.org/git/[e-mail]/

AbdAlRahman Gad Aug. 13, 2024, 9:58 p.m. UTC | #2

On 8/13/24 10:57 PM, Eric Sunshine wrote:
> On Thu, Aug 8, 2024 at 12:34 PM AbdAlRahman Gad <[e-mail]> wrote:
>> - 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.
>> [...]
>> 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.
> 
> Thanks. This sort of information -- explaining the aim of the series
> and explaining what will be done later -- is appreciated by reviewers.
> 
> There are a few additional pieces of information you can include in
> the cover letter to make life even simpler for reviewers:
> 
> * provide a link to the previous version of the series
> 
> * explain what differs in this version as compared with the previous version
> 
> * include a range-diff showing the actual changes between this version
> and the previous version (see `git format-patch --range-diff=`
> 
> There are many good examples on the mailing which illustrate the above
> points; here is one such instance:
> https://lore.kernel.org/git/[e-mail]/

Thanks! I'll add this to my notes and start doing it in the next patch.