On Tue, Feb 14, 2023 at 8:09 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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
