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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 10, 2023 at 3:05 AM Vinayak Dev <vinayakdev.sci@xxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxx/
[2]: https://lore.kernel.org/git/CAPig+cT1EtPO2FLvTsw3SjgCgk=ovNwY77hsX6p7ETKiq8aNog@xxxxxxxxxxxxxx/



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux