Hi Junio,
On Tue, 13 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
> > @@ -1292,7 +1316,12 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
> > struct todo_item *item = todo_list->items + todo_list->current;
> > if (save_todo(todo_list, opts))
> > return -1;
> > - res = do_pick_commit(item->command, item->commit, opts);
> > + if (item->command <= TODO_REVERT)
> > + res = do_pick_commit(item->command, item->commit,
> > + opts);
> > + else if (item->command != TODO_NOOP)
> > + return error(_("unknown command %d"), item->command);
>
> I wonder if making this a switch() statement is easier to read in
> the longer run. The only thing at this point we are gaining by "not
> only mapping and enum must match, the orders matter" is so that this
> codepath can do the same thing for PICK and REVERT, but these two
> would become more and more minority as we learn more words.
I doubt that this is easier to read. There are essentially three
categories we are handling: exec, comments, and everything else. IMO the
current code is the easiest to understand.
Ciao,
Dscho