Re: [PATCH v6 05/15] sequencer: introduce the `merge` command

From: Phillip Wood <phillip.wood@talktalk.net>
Date: Apr 18, 2018, 18:04 UTC
Message-ID: <66b0b2e6-11b6-20f9-da6b-211c9c4dc758@talktalk.net>
In-Reply-To: <nycvar.QRO.7.76.6.1804140248110.234@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

On 14/04/18 01:51, Johannes Schindelin wrote:

Show 18 quoted lines

> Hi Phillip,
> 
> On Fri, 13 Apr 2018, Phillip Wood wrote:
> 
>> On 13/04/18 11:12, Phillip Wood wrote:
>>> @@ -3030,7 +3029,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>>>  			return error(_("unknown command %d"), item->command);
>>>  
>>>  		if (res < 0 && (item->command == TODO_LABEL ||
>>> -				item->command == TODO_RESET)) {
>>> +				item->command == TODO_RESET ||
>>> +				item->command == TODO_MERGE)) {
>>
>> Unfortunately it's not as simple as that - we only want to reschedule if
>> merge_recursive() fails, not if run_git_commit() does.
> 
> Correct. How about introducing a flag `reschedule` that is passed to
> do_label(), do_reset() and do_merge()?

That would work (I was thinking about using return codes but having a parameter is a better idea). Do you want me to re-roll the fixups or are you happy to make the changes in your next version?

> 
> Seeing as do_reset() and do_merge() already have a replay_opts parameter,
> we could add a field `needs_rescheduling` and pass the replay_opts also to
> do_label().

I'm slightly wary of putting state in an options structure but maybe it doesn't matter.

Best Wishes

Phillip

> Ciao,
> Dscho
>