Re: [PATCH 04/15] sequencer: lib'ify prepare_revs()

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 26, 2016, 13:40 UTC
Message-ID: <alpine.DEB.2.20.1608261534110.4924@virtualbox>
In-Reply-To: <xmqqinuojwc4.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Thu, 25 Aug 2016, Junio C Hamano wrote:

Show 15 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> 
> I am still looking at sequencer.c in 'master', but I do not think
> that the sole caller of this function, walk_revs_populate_todo(),
> is prepared to act on an error return from this function and instead
> it expects this to die() when in trouble.  And I do not think I saw
> the function touched in the steps so far.
> 
> So this step smells like a fishy conversion to me.

The fishy part is, of course, that I managed not to realize that the patch libifiying walk_revs_populate_todo() came *after* this patch that libified one of its callees, prepare_revs().

FWIW the same is true for sequencer_pick_revisions(), which was libified way too late in the game.

This is once again a demonstration that a final patch series does *not* reflect the process of developing it: in this case, the order of the patches is the *opposite* of their development, as I libified functions from the lowest, deepest call depth up to the top-level functions.

Fixed in v2, Dscho