On Fri, 10 Feb 2023 at 16:38, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > 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/ Sorry for this error, I actually sent the patch before I got your review of the prior one. > > 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. This is correct, I will make the message more informative about the relevant aspects of the changes. > > 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. I apologise for this mistake again. > 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). If by testing you mean running the test file to ensure there are no errors, I did do it thoroughly before sending the patch, so I can vouch for the changes :) Thanks a lot! Vinayak
