| Message ID | 20230202171821.10508-4-cheskaqiqi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t4113: modernize test style | expand |
Shuqi Liang <cheskaqiqi@gmail.com> writes: > use "<<-" instead of "<<" You forgot to explain "Why?". What you did, anybody can see in the patch text. Why you did so is what you are expected to explain in your proposed log message. > -cat >test-patch <<\EOF > +cat >test-patch <<-\EOF > diff --git a/file b/file > --- a/file > +++ b/file There is no need to do this, as the body of the here-doc is not indented/prefixed with HT at all. > @@ -31,7 +31,7 @@ test_expect_success 'apply at the end' ' > ' > > test_expect_success 'setup for apply at the beginning' ' > - cat >test-patch <<\EOF > + cat >test-patch <<-\EOF > diff a/file b/file > --- a/file > +++ b/file This is necessary but that is only because [PATCH v2 2/4] broke it. In general, we frown upon a series where a bug is introduced in an earlier step, with another patch fixing that bug. Let's not introduce such a bug that we need to fix later from the beginning instead. Thanks.
On Thu, Feb 2, 2023 at 4:05 PM Junio C Hamano <gitster@pobox.com> wrote: > You forgot to explain "Why?". What you did, anybody can see in the > patch text. Why you did so is what you are expected to explain in > your proposed log message. Thanks. I did make a lot of mistakes in writing a good commit message. I will modify it. > > -cat >test-patch <<\EOF > > +cat >test-patch <<-\EOF > > diff --git a/file b/file > > --- a/file > > +++ b/file > > There is no need to do this, as the body of the here-doc is not > indented/prefixed with HT at all. yeah, I did not notice that , According to t/README says, Indent the body of here-document, and use "<<-" instead of "<<" to strip leading TABs used for indentation. But here do not have the leading TABS in front of it. > > @@ -31,7 +31,7 @@ test_expect_success 'apply at the end' ' > > ' > > > > test_expect_success 'setup for apply at the beginning' ' > > - cat >test-patch <<\EOF > > + cat >test-patch <<-\EOF > > diff a/file b/file > > --- a/file > > +++ b/file > > This is necessary but that is only because [PATCH v2 2/4] broke it. > In general, we frown upon a series where a bug is introduced in an > earlier step, with another patch fixing that bug. > > Let's not introduce such a bug that we need to fix later from the > beginning instead. Thanks, I will introduce the new bug caused by the current patch in the beginning. -------- Thanks, shuqi
diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh index d84f632bc3..d5b15e97d3 100755 --- a/t/t4113-apply-ending.sh +++ b/t/t4113-apply-ending.sh @@ -8,7 +8,7 @@ test_description='git apply trying to add an ending line. ' . ./test-lib.sh -cat >test-patch <<\EOF +cat >test-patch <<-\EOF diff --git a/file b/file --- a/file +++ b/file @@ -31,7 +31,7 @@ test_expect_success 'apply at the end' ' ' test_expect_success 'setup for apply at the beginning' ' - cat >test-patch <<\EOF + cat >test-patch <<-\EOF diff a/file b/file --- a/file +++ b/file
use "<<-" instead of "<<" Signed-off-by: Shuqi Liang <cheskaqiqi@gmail.com> --- t/t4113-apply-ending.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)