| Message ID | 20230221214653.85830-2-gvivan6@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GSOC,v2,1/1] t4121: modernize test style | expand |
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, therefore update the tests to adhere to the new > style. The new commit message is sufficiently descriptive, thanks for updating. In terms of readability, it is a bit of a run-on sentence (the comma after "lines" could be a period, i.e. "...separate lines. Therefore, update the..."). I don't think it needs to be updated, but it's something to keep in mind for future contributions. :) > > Signed-off-by: Vivan Garg <gvivan6@gmail.com> > --- > 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..f1cc42ff71 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 > +' > > -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 > +' Whitespace looks good here. I think this is ready-to-merge; thanks! > > test_done
On Tue, Feb 21, 2023 at 3:17 PM Victoria Dye <vdye@github.com> wrote: > The new commit message is sufficiently descriptive, thanks for updating. In > terms of readability, it is a bit of a run-on sentence (the comma after > "lines" could be a period, i.e. "...separate lines. Therefore, update > the..."). I don't think it needs to be updated, but it's something to keep > in mind for future contributions. :) I've taken note of it. Thanks! > Whitespace looks good here. I think this is ready-to-merge; thanks! Thanks again for the review!
diff --git a/t/t4121-apply-diffs.sh b/t/t4121-apply-diffs.sh index a80cec9d11..f1cc42ff71 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 +' -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 +' test_done
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, therefore update the tests to adhere to the new style. Signed-off-by: Vivan Garg <gvivan6@gmail.com> --- t/t4121-apply-diffs.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)