Re: [Outreachy-Microproject][PATCH 1/1] t0000: replace 'test -[def]' with helpers

[Shourya Shukla <shouryashukla.oo@gmail.com>]

Hello Caleb,

I have some comments.

First of all, I notice that this is a v2 of this PATCH:
https://lore.kernel.org/git/20201018005522.217397-1-caleb.tillman@gmail.com/

So, I think that the subject of the mail should reflect the same. I
believe that you have used 'git format-patch' to generate this mail
therefore what you can do is:

'git format-patch -v2 @~n', where 'n' is the number of commits which you
want to include in the patch. So in your case it will be:
'git format-patch -v2 @~1' and a patch mail will be generated.

Also, you need not put the '[Outreachy-Microproject]' tag in the
subject, '[OUTREACHY]' will suffice.

Now, coming to the meat of the patch.

> The test_path_is* functions provide debug-friendly upon failure.

This commit can be redone to be even more better. This does not exactly
reflect what has been done. I understand that yes 'test_patch_is_*'
functions are better and why they are better. But where did you replace
them, this is left unanswered.

This is one example of how the commit messages can be, not too verbose
and not too short, somewhere in the middle:
https://lore.kernel.org/git/20200118083326.9643-6-shouryashukla.oo@gmail.com/

> Signed-off-by: Caleb Tillman <caleb.tillman@gmail.com>
---
> Outreachy microproject, revised submission.
>  t/t0000-basic.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 923281af93..eb99892a87 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -1191,7 +1191,7 @@ test_expect_success 'writing this tree with --missing-ok' '
>  test_expect_success 'git read-tree followed by write-tree should be idempotent' '
> 	rm -f .git/index &&
> 	git read-tree $tree &&
> -	test -f .git/index &&
> +	test_path_is_file .git/index &&
>  	newtree=$(git write-tree) &&
> 	test "$newtree" = "$tree"

The change is fine but I feel you can easily find files in which you can
do the same type of change but in a large quantity. This way you will
get an even better idea of how the tests work at Git. To find such
files, one way can be to look here:
https://github.com/git/git/tree/master/t

Here if you try finding files which had commits over 11-12+ years ago,
you will find some ancient relics to modernise too! Great that you took
Taylor's advice ;)

Best of luck,
Shourya Shukla