Re: [PATCH v4 6/5] fixup! rebase: add --reset-author-date
Hi dscho and Danh
On 29/05/2020 03:59, Johannes Schindelin wrote:
Show 56 quoted lines
> 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
>