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

Re: [RFC][GSoC][PATCH] t9160: Change test -(d | f) to test_path_is_*

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Feb 10, 2023, 11:08 UTC
Message-ID
<CAPig+cS-9gbxb3tWqaWtsjfiQR60GQXpfCKM7-neu6AH8rUvcA@mail.gmail.com>
In-Reply-To
<20230210080110.32983-1-vinayakdev.sci@gmail.com>
On Fri, Feb 10, 2023 at 3:05 AM Vinayak Dev <vinayakdev.sci@gmail.com> wrote:
> Changes test -d and test -f commands in
s/Changes/Change/
> t/t9160-git-svn-preserve-empty-dirs.sh to test_path_is_dir and
> test_path_is_file respectively, which are helper functions defined in
> t/test-lib-functions.sh

This summarizes the changes made by the patch, but readers also want to know _why_ the changes are being made; in fact, "why" is more important. Previous patches similar to this one have explained that test_path_is_dir() and test_path_is_file() are superior to `test -d` and `test -f` because the functions provide diagnostic output when they fail, and that diagnostic output can make it easier to debug a failing tests. So, if you reroll the patch, focus on explaining the benefits of the functions rather than explaining the mechanical changes made by the patch.

> Signed-off-by: Vinayak Dev <vinayakdev.sci@gmail.com>
>
> vinayakdsci (1):
>   Change test -(d | f) to corresponding test_path_*

As with the other GSoC patch you submitted[1], this one is also missing the "---" line below your sign-off, which tells git-am where the commit message ends. As mentioned in [2] you may need to adjust your tools or workflow to prevent the "---" line from being stripped.

The actual changes made by the patch are probably reasonable (though I don't have CVS libraries installed presently, so I wasn't able to actually test the changes).

[1]: https://lore.kernel.org/git/20230209164552.8971-1-vinayakdev.sci@gmail.com/ [2]: https://lore.kernel.org/git/CAPig+cT1EtPO2FLvTsw3SjgCgk=ovNwY77hsX6p7ETKiq8aNog@mail.gmail.com/

Previous: Vinayak DevNext: Vinayak Dev
Message 2 of 4 in “t9160: Change test -(d | f) to test_path_is_*”
  1. Vinayak DevFeb 10, 2023
  2. Eric SunshineFeb 10, 2023
  3. Vinayak DevFeb 10, 2023
  4. Eric SunshineFeb 14, 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.