Hi Phillip,
On Fri, 20 Apr 2018, Phillip Wood wrote:
> On 19/04/18 13:20, Johannes Schindelin wrote:
>
> [... please cull long stretches of quoted mail that is not responded to ...]
>
> > @@ -2665,6 +2846,12 @@ static int pick_commits(struct todo_list *todo_list,
> > struct replay_opts *opts)
> > /* `current` will be incremented below */
> > todo_list->current = -1;
> > }
> > + } else if (item->command == TODO_LABEL) {
> > + if ((res = do_label(item->arg, item->arg_len)))
> > + goto reschedule;
>
> I can see why you've implemented like this but I'm uneasy with jumping
> into a block guarded with "if (item->command <= TODO_SQUASH)" when
> item->command > TODO_SQUASH. I think it works OK at the moment but it's
> possible that in the future someone will edit that block of code and add
> something like
>
> if (item->command == TODO_PICK)
> do_something()
> else
> do_something_else()
>
> assuming that item->command <= TODO_SQUASH because they haven't noticed
> the goto jumping back into that block.
I changed it by duplicating the rescheduling, as I agree that it is
somewhat dangerous what with all the code going on after the rescheduling
of a pick/fixup/squash/reword.
My plan is to go over the documentation changes once more tomorrow, with a
fresh set of eyes, and then submit the hopefully final iteration of this
patch series.
Ciao,
Dscho
