From: AbdAlRahman Gad Date: Thu, 08 Aug 2024 16:18:43 GMT Subject: Re: [PATCH v5 0/8] [Newcomer] t7004: modernize the style Message-ID: <485c3474-992f-41f3-aaaf-0ad1d43464c4@gmail.com> In-Reply-To: On 8/8/24 18:42, Junio C Hamano wrote: > AbdAlRahman Gad 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. > Thanks! I'll send a v6 right away. >> 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. I'll work on them the next patch series.