Re: [PATCH v3 0/2] rebase -i: extend rebase.missingCommitsCheck

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

Hi Alban

On 05/12/2019 23:15, Alban Gruin wrote:
> Hi Junio,
> 
> Le 04/12/2019 à 22:51, Junio C Hamano a écrit :
>> Alban Gruin <alban.gruin@gmail.com> writes:
>>
>>> To prevent mistakes when editing a branch, rebase features a knob,
>>> rebase.missingCommitsCheck, to warn the user if a commit was dropped.
>>> Unfortunately, this check is only effective for the initial edit, which
>>> means that if you edit the todo list at a later point of the rebase and
>>> drop a commit, no warnings or errors would be issued.
>>> ...
>>>   rebase-interactive.c          | 57 ++++++++++++++++++++----
>>>   rebase-interactive.h          |  2 +
>>>   sequencer.c                   | 53 ++++++----------------
>>>   sequencer.h                   |  1 -
>>>   t/t3404-rebase-interactive.sh | 83 +++++++++++++++++++++++++++++++++++
>>>   5 files changed, 147 insertions(+), 49 deletions(-)
>>
>> This passes the self-test when tested by itself, but when merged
>> near the tip of 'pu', it breaks t3404.116, it seems.
>>
> 
> After a quick investigation, it comes from
> pw/post-commit-from-sequencer.  Since then, tests are expected to setup
> the editor and run the commands using it in a subshell.  So the fix is
> straightforward.
> 
> Perhaps I should take ag/sequencer-todo-updates, merge
> pw/post-commit-from-sequencer, rebase this series onto the result, fix
> the issue, and reroll the series?

If the issue is just using a subshell to set the editor then (assuming 
you're only adding new tests) I don't think you need to rebase - just 
change your tests and it should be fine when Junio merges it into pu. 
I'm sorry I've not looked at the latest version yet, I'll try and get 
round to it next week.

Best Wishes

Phillip
> Cheers,
> Alban
>