From: Jakub Narębski <jnareb@gmail.com>
Date: Wed, 31 Aug 2016 18:24:16 GMT
Subject: Re: [PATCH 11/22] sequencer: get rid of the subcommand field
Message-ID: <2ba846cc-0e0f-fc7a-27d1-3b8ad66ea72b@gmail.com>
In-Reply-To: <258635d3aa7f70cb1b20ea722e10ad439406b31e.1472457609.git.johannes.schindelin@gmx.de>

W dniu 29.08.2016 o 10:05, Johannes Schindelin pisze:

> The subcommands are used exactly once, at the very beginning of
> sequencer_pick_revisions(), to determine what to do. This is an
> unnecessary level of indirection: we can simply call the correct
> function to begin with. So let's do that.

Looks good.  Parsing is moved from parse_args(), now unnecessary,
to the new run_sequencer().  Which also picked up dispatch from
sequencer_pick_revisions() - that sometimes didn't pick revisions :-o.

"All problems in computer science can be solved by another level
 of indirection, except of course for the problem of too many
 indirections." -- David John Wheeler

> 
> While at it, ensure that the subcommands return an error code so that
> they do not have to die() all over the place (bad practice for library
> functions...).

This perhaps should be moved to a separate patch, but I guess
there is a reason behind "while at it".

Also subcommand functions no longer are local to sequencer.c

> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/revert.c | 36 ++++++++++++++++--------------------
>  sequencer.c      | 35 +++++++++++------------------------
>  sequencer.h      | 13 ++++---------
>  3 files changed, 31 insertions(+), 53 deletions(-)

Nice size reduction.