| Message ID | 20200527175748.54468-1-phillip.wood123@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 2020-05-27 18:57:48+0100, Phillip Wood <phillip.wood123@gmail.com> wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Sorry I somehow forgot to commit this before sending the v4 patches, > it fixes up the final patch > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > t/t3436-rebase-more-options.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh > index 5ee193f333..ecfd68397f 100755 > --- a/t/t3436-rebase-more-options.sh > +++ b/t/t3436-rebase-more-options.sh > @@ -196,7 +196,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' > git rebase --apply --ignore-date HEAD^ && > git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && > git rebase -m --ignore-date HEAD^ && > - git log -2 --pretty="format:%ai" >authortime && > + git log -2 --pretty=%ai >authortime && > grep "+0000" authortime >output && > test_line_count = 2 output > ' This version addressed all of my concerns, LGTM. Only the last test_line_count = 2 output puzzled me at first. Since it's the only usage of test_line_count in this version Turn out, it's equivalence with: -----------8<----------- diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh index ecfd68397f..abe9af4d8c 100755 --- a/t/t3436-rebase-more-options.sh +++ b/t/t3436-rebase-more-options.sh @@ -197,8 +197,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && git rebase -m --ignore-date HEAD^ && git log -2 --pretty=%ai >authortime && - grep "+0000" authortime >output && - test_line_count = 2 output + ! grep -v "+0000" authortime ' # This must be the last test in this file ------>8------ This is the check used in t3436.15 Current version is good as is, anyway.
Hi, On Thu, 28 May 2020, Đoàn Trần Công Danh wrote: > On 2020-05-27 18:57:48+0100, Phillip Wood <phillip.wood123@gmail.com> wrote: > > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > > > Sorry I somehow forgot to commit this before sending the v4 patches, > > it fixes up the final patch > > > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > > --- > > t/t3436-rebase-more-options.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh > > index 5ee193f333..ecfd68397f 100755 > > --- a/t/t3436-rebase-more-options.sh > > +++ b/t/t3436-rebase-more-options.sh > > @@ -196,7 +196,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' > > git rebase --apply --ignore-date HEAD^ && > > git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && > > git rebase -m --ignore-date HEAD^ && > > - git log -2 --pretty="format:%ai" >authortime && > > + git log -2 --pretty=%ai >authortime && > > grep "+0000" authortime >output && > > test_line_count = 2 output > > ' > > This version addressed all of my concerns, LGTM. > > Only the last > > test_line_count = 2 output > > puzzled me at first. > Since it's the only usage of test_line_count in this version > Turn out, it's equivalence with: > -----------8<----------- > diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh > index ecfd68397f..abe9af4d8c 100755 > --- a/t/t3436-rebase-more-options.sh > +++ b/t/t3436-rebase-more-options.sh > @@ -197,8 +197,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' > git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && > git rebase -m --ignore-date HEAD^ && > git log -2 --pretty=%ai >authortime && > - grep "+0000" authortime >output && > - test_line_count = 2 output > + ! grep -v "+0000" authortime > ' > > # This must be the last test in this file > ------>8------ Good suggestion! I've read through all 5 patches, and rather than repeating much of what I said about 1/5 and 2/5 in 4/5, I'll just say it here that it applies there, too: less repetitions in the test script, and I'd prefer the layer where the `apply` vs `merge` options are set to be `cmd__rebase()` rather than `run_am()` (and `get_replay_opts()`). All in all, it was a pleasant read. Thanks, Dscho
Hi dscho and Danh On 29/05/2020 03:59, Johannes Schindelin wrote: > Hi, > > On Thu, 28 May 2020, Đoàn Trần Công Danh wrote: > >> On 2020-05-27 18:57:48+0100, Phillip Wood <phillip.wood123@gmail.com> wrote: >>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>> >>> Sorry I somehow forgot to commit this before sending the v4 patches, >>> it fixes up the final patch >>> >>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >>> --- >>> t/t3436-rebase-more-options.sh | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh >>> index 5ee193f333..ecfd68397f 100755 >>> --- a/t/t3436-rebase-more-options.sh >>> +++ b/t/t3436-rebase-more-options.sh >>> @@ -196,7 +196,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' >>> git rebase --apply --ignore-date HEAD^ && >>> git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && >>> git rebase -m --ignore-date HEAD^ && >>> - git log -2 --pretty="format:%ai" >authortime && >>> + git log -2 --pretty=%ai >authortime && >>> grep "+0000" authortime >output && >>> test_line_count = 2 output >>> ' >> >> This version addressed all of my concerns, LGTM. >> >> Only the last >> >> test_line_count = 2 output >> >> puzzled me at first. >> Since it's the only usage of test_line_count in this version >> Turn out, it's equivalence with: >> -----------8<----------- >> diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh >> index ecfd68397f..abe9af4d8c 100755 >> --- a/t/t3436-rebase-more-options.sh >> +++ b/t/t3436-rebase-more-options.sh >> @@ -197,8 +197,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' >> git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && >> git rebase -m --ignore-date HEAD^ && >> git log -2 --pretty=%ai >authortime && >> - grep "+0000" authortime >output && >> - test_line_count = 2 output >> + ! grep -v "+0000" authortime >> ' >> >> # This must be the last test in this file >> ------>8------ > > Good suggestion! Yes thanks Danh I'll update with that > I've read through all 5 patches, and rather than repeating much of what I > said about 1/5 and 2/5 in 4/5, I'll just say it here that it applies > there, too: less repetitions in the test script, I'll look at adding a helper for to do the checks > and I'd prefer the layer > where the `apply` vs `merge` options are set to be `cmd__rebase()` rather > than `run_am()` (and `get_replay_opts()`). Yeah that would make it nicer. > All in all, it was a pleasant read. Thanks for your comments on this series Phillip > Thanks, > Dscho >
diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh index 5ee193f333..ecfd68397f 100755 --- a/t/t3436-rebase-more-options.sh +++ b/t/t3436-rebase-more-options.sh @@ -196,7 +196,7 @@ test_expect_success '--ignore-date is an alias for --reset-author-date' ' git rebase --apply --ignore-date HEAD^ && git commit --allow-empty -m empty --date="$GIT_AUTHOR_DATE" && git rebase -m --ignore-date HEAD^ && - git log -2 --pretty="format:%ai" >authortime && + git log -2 --pretty=%ai >authortime && grep "+0000" authortime >output && test_line_count = 2 output '