git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Shuqi Liang <cheskaqiqi@gmail.com>
Date
Feb 15, 2023, 02:40 UTC
Message-ID
<CAMO4yUHx68jSe31gKon6_hXTiqufHnHAFkP32DXmWg7dtagAfQ@mail.gmail.com>
In-Reply-To
<CAPig+cTKCMwCQ2TxXyc1KquZmB0QWQAuuag52a1qmeYh6XPXYQ@mail.gmail.com>
On Tue, Feb 14, 2023 at 8:09 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 10 quoted lines
> 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.
Show 10 quoted lines
> 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 &&
Show 5 quoted lines
> 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

Previous: Eric SunshineNext: Shuqi Liang
Message 44 of 49 in “t/t4113-apply-ending.sh: Modernize a test script”
  1. Shuqi LiangJan 31, 2023
  2. Andrei RybakFeb 1, 2023
  3. cheska franFeb 2, 2023
  4. 0/4 t4113: modernize test styleShuqi Liang, Feb 2, 2023
  5. 1/4 t4113: replace backslash with single quoteShuqi Liang, Feb 2, 2023
  6. Junio C HamanoFeb 2, 2023
  7. Shuqi LiangFeb 5, 2023
  8. 2/4 t4113:put second creation in own stepShuqi Liang, Feb 2, 2023
  9. 3/4 t4113: use "<<-" instead of "<<"Shuqi Liang, Feb 2, 2023
  10. Junio C HamanoFeb 2, 2023
  11. Shuqi LiangFeb 5, 2023
  12. 4/4 t4113: indent with tabsShuqi Liang, Feb 2, 2023
  13. Junio C HamanoFeb 2, 2023
  14. Shuqi LiangFeb 5, 2023
  15. 0/3 modernize styleShuqi Liang, Feb 5, 2023
  16. 1/3 t4113: modernize a test scriptShuqi Liang, Feb 5, 2023
  17. 2/3 t4113: put executable lines to test_expect_successShuqi Liang, Feb 5, 2023
  18. 3/3 t4113: indent with spaceShuqi Liang, Feb 5, 2023
  19. Eric SunshineFeb 5, 2023
  20. Shuqi LiangFeb 6, 2023
  21. 0/3 t4113: modernize styleShuqi Liang, Feb 6, 2023
  22. 1/3 t4113: modernize test scriptShuqi Liang, Feb 6, 2023
  23. 2/3 t4113: indent with tabShuqi Liang, Feb 6, 2023
  24. 3/3 t4113: put executable lines to test_expect_successShuqi Liang, Feb 6, 2023
  25. Ævar Arnfjörð BjarmasonFeb 6, 2023
  26. Junio C HamanoFeb 6, 2023
  27. Shuqi LiangFeb 6, 2023
  28. Ævar Arnfjörð BjarmasonFeb 7, 2023
  29. Shuqi LiangFeb 7, 2023
  30. Shuqi LiangFeb 8, 2023
  31. Ævar Arnfjörð BjarmasonFeb 8, 2023
  32. Shuqi LiangFeb 10, 2023
  33. Eric SunshineFeb 15, 2023
  34. Eric SunshineFeb 15, 2023
  35. Shuqi LiangFeb 14, 2023
  36. Junio C HamanoFeb 14, 2023
  37. 0/3 t4113: modernize styleShuqi Liang, Feb 9, 2023
  38. 1/3 t4113: modernize test scriptShuqi Liang, Feb 9, 2023
  39. 2/3 t4113: indent with tabShuqi Liang, Feb 9, 2023
  40. Eric SunshineFeb 15, 2023
  41. Shuqi LiangFeb 15, 2023
  42. 3/3 t4113: put executable lines to test_expect_successShuqi Liang, Feb 9, 2023
  43. Eric SunshineFeb 15, 2023
  44. Shuqi LiangFeb 15, 2023
  45. 0/3 t4113: modernize styleShuqi Liang, Feb 15, 2023
  46. 0/3 t4113: modernize styleShuqi Liang, Feb 15, 2023
  47. 1/3 t4113: modernize test scriptShuqi Liang, Feb 15, 2023
  48. 2/3 t4113: indent with tabShuqi Liang, Feb 15, 2023
  49. 3/3 t4113: put executable lines to test_expect_successShuqi Liang, Feb 15, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.