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@xxxxxxxxx> wrote: > > From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxx> > > --- > > 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
