On Tue, Feb 14, 2023 at 8:09 PM Eric Sunshine [off-list ref] 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