Re: [GSOC][PATCH] t4121: modernize test style

[Victoria Dye <vdye@github.com>]

Vivan Garg wrote:
> Test scripts in file t4121-apply-diffs.sh are written in old style,
> where the test_expect_success command and test title are written on
> separate lines

nit: period at the end of the sentence (s/lines/lines.)

Also, this commit message explains *why* you're making the change, but not
what the commit actually does (that is, update the tests to adhere to the
new style). Would you mind adding a note about that to the message?

> 
> Signed-off-by: Vivan Garg <gvivan6@gmail.com>
> ---
> Greetings, my name is Vivan Garg. I am currently pursuing a double major
> in computer science and finance at the University of Waterloo in Canada. 
> I am currently completing my third software developer internship at Morgan
> Stanley. As part of my coursework, I studied C and shell scripting, which 
> I then applied in internships and personal projects. C++ is the programming 
> language that I am most comfortable with right now. Please feel free to 
> address me as Vivan, and my pronouns are he/him/his. I meet the requirements 
> for GSOC participation. So far, I've either read or skimmed the following 
> documents based on prior knowledge: Submitting patches, Coding guidelines, 
> Myfirstcontribution.txt, gittutorial, Giteveryday, readme, Hacking-Git, 
> General-Microproject-Information, SoC-2023-Ideas, and 
> General-Application-Information. I'm looking forward to having a fantastic 
> time here! 

Welcome to the Git community, and thanks for your contribution! :) 

> 
>  t/t4121-apply-diffs.sh | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t4121-apply-diffs.sh b/t/t4121-apply-diffs.sh
> index a80cec9d11..2ff38ededa 100755
> --- a/t/t4121-apply-diffs.sh
> +++ b/t/t4121-apply-diffs.sh
> @@ -16,8 +16,8 @@ echo '1
>  7
>  8' >file
>  
> -test_expect_success 'setup' \
> -	'git add file &&
> +test_expect_success 'setup' '
> +	git add file &&
>  	git commit -q -m 1 &&
>  	git checkout -b test &&
>  	mv file file.tmp &&
> @@ -27,10 +27,11 @@ test_expect_success 'setup' \
>  	git commit -a -q -m 2 &&
>  	echo 9 >>file &&
>  	git commit -a -q -m 3 &&
> -	git checkout main'
> +	git checkout main
> +'

This test looks good.

>  
> -test_expect_success \
> -	'check if contextually independent diffs for the same file apply' \
> -	'( git diff test~2 test~1 && git diff test~1 test~0 )| git apply'
> +test_expect_success 'check if contextually independent diffs for the same file apply' '
> +	( git diff test~2 test~1 && git diff test~1 test~0 )| git apply
> +'

As for this one, the test is correctly updated to the new style (per the
microproject prompt). However, the spacing around the '|' is a little weird
- I think there should be a space after ')'. On your next re-roll, could you
fix that spacing (in this patch is fine - it's not a substantial enough
change to warrant its own commit)?

>  
>  test_done