Re: [PATCH v4 6/5] fixup! rebase: add --reset-author-date

[Phillip Wood <phillip.wood123@gmail.com>]

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
>