Re: [PATCH v5 3/3] t4113: put executable lines to test_expect_success

From: Shuqi Liang <cheskaqiqi@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>, git@vger.kernel.org
Subject: Re: [PATCH v5 3/3] t4113: put executable lines to test_expect_success
Date: Tue, 14 Feb 2023 21:40:33 -0500	[thread overview]
Message-ID: <CAMO4yUHx68jSe31gKon6_hXTiqufHnHAFkP32DXmWg7dtagAfQ@mail.gmail.com> (raw)
In-Reply-To: <CAPig+cTKCMwCQ2TxXyc1KquZmB0QWQAuuag52a1qmeYh6XPXYQ@mail.gmail.com>

On Tue, Feb 14, 2023 at 8:09 PM Eric Sunshine <sunshine@sunshineco.com> wrote:

> Although it's true that t/README explains why code should be placed
> inside tests, you can help readers out by simply explaining the reason
> here in the commit message. For instance, you might replace the above
> paragraph with:
>
>     Some old test scripts have setup code outside of tests. This
>     is problematic since any failures of the setup code will go
>     unnoticed. Therefore, move setup code into the tests themselves
>     so that failures are properly flagged.
>
Thanks, that really makes the change more clear for the readers. I learn a lot.

> Reviewers appreciate well-explained commit messages, but they also
> appreciate succinctness. Although it may not always be obvious how
> much to write in a commit message, you can assume that reviewers will
> understand obvious changes simply by reading the patch itself, thus
> you don't need to mention every little detail in the commit message.
> The important thing to mention in the commit message is the
> explanation of _why_ the change is being made, plus any changes which
> might not be obvious. In this case, all the changes are obvious, so,
> really, you can collapse this entire commit message to just the first
> paragraph.

Yeah, the commit looks very wordy. I'll make it more succinct.
>     test_write_lines a b c >file &&

> Same comment about simply using double quotes instead of
> single-quotes, however, this is also another really good place to use
> test_write_lines:
>
>     test_write_lines a b c >file &&

Thanks for the tips!

(Sorry for sending the V6 to you twice I send it by accident .)

-------------------------
Thanks
Shuqi