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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 19, 2018, 12:03 UTC
Message-ID: <nycvar.QRO.7.76.6.1804191402060.4241@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <nycvar.QRO.7.76.6.1804140248110.234@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

Hi Phillip,

On Sat, 14 Apr 2018, Johannes Schindelin wrote:

Show 20 quoted lines

> 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()?
> 
> 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().

Nevermind, we already use the trick in do_pick_commit() that -1 means: reschedule, 0 means: success, and 1 means: merge conflicts (don't bother rescheduling).

It just had slipped my mind; I use the same convention in do_merge() now.

Thank you so much for your review and suggestions. I *think* I incorporated it all.

Ciao, Dscho