Re: [PATCH v2 03/14] sequencer: lib'ify write_message()

2016-08-30 Thread Johannes Schindelin
Hi Junio,

On Mon, 29 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Instead of dying there, let the caller high up in the callchain
> > notice the error and handle it (by dying, still).
> >
> > The only caller of write_message(), do_pick_commit() already checks
> > the return value and passes it on to its callers, so its caller must
> > be already prepared to handle error returns, and with this step, we
> > make it notice an error return from this function.
> > ...
> > @@ -223,7 +226,7 @@ static int fast_forward_to(const unsigned char *to, 
> > const unsigned char *from,
> >  
> > read_cache();
> > if (checkout_fast_forward(from, to, 1))
> > -   exit(128); /* the callee should have complained already */
> > +   return -1; /* the callee should have complained already */
> 
> This hunk does not seem to have anything to do with write_message()
> conversion, other than that its only caller, do_pick_commit(), also
> calls write_message().

Darn. Sorry that this slipped through. I split it out into its own commit,
and fixed checkout_fast_forward_to() in yet another commit.

Ciao,
Dscho


Re: [PATCH v2 03/14] sequencer: lib'ify write_message()

2016-08-29 Thread Junio C Hamano
Johannes Schindelin  writes:

> Instead of dying there, let the caller high up in the callchain
> notice the error and handle it (by dying, still).
>
> The only caller of write_message(), do_pick_commit() already checks
> the return value and passes it on to its callers, so its caller must
> be already prepared to handle error returns, and with this step, we
> make it notice an error return from this function.
> ...
> @@ -223,7 +226,7 @@ static int fast_forward_to(const unsigned char *to, const 
> unsigned char *from,
>  
>   read_cache();
>   if (checkout_fast_forward(from, to, 1))
> - exit(128); /* the callee should have complained already */
> + return -1; /* the callee should have complained already */

This hunk does not seem to have anything to do with write_message()
conversion, other than that its only caller, do_pick_commit(), also
calls write_message().  checkout_fast_forward() itself can die when
it cannot write the index, though, so this hunk may deserve to be in
its own patch that teaches checkout_fast_forward() to instead return
an error if/when its caller desires.

With the updated message, the series has become far easier to review,
and the reordering them to sequencer-pick-revisions at the beginning
makes quite a lot of sense.