Re: [PATCH 06/15] sequencer: lib'ify read_populate_todo()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

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

> 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>
> > ---
> >  sequencer.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> Instead of dying there, you let the caller high up in the callchain
> to notice the error and handle it (by dying).
> 
> The only caller of read_populate_todo(), sequencer_continue() can
> already return errors, so its caller must be already prepared to
> handle error returns, and with this step, you make it notice an
> error return from this function.  So this is a safe conversion to
> make read_populate_todo() callable from new callers that want it not
> to die, without changing the external behaviour of anything
> existing.
> 
> Good.
> 
> By the way, I am writing these as review comments because I do not
> want to keep repeating this kind of analysis as a reviewer.  I am
> demonstrating what should have been in the commit log message
> instead, so that the reviewer does not have to spend extra time, if
> the reviewer trusts the author's diligence well enough, to see if
> the conversion makes sense.
> 
> Please follow the example when/if you have to reroll.  I want the
> patches to show the evidence of careful analysis to reviewers so
> that they can gauge the trustworthiness of the patches.  With this
> round of patches, honestly, I cannot tell if it is a mechanical
> substitution alone, or such a substitution followed by a careful
> verification of the callers.

Duly noted.

I fixed up the patch order as well as the commit messages.

Now on to a request of my own: I am once again reminded that I have *no*
good mail client to serve me. (Before you suggest it: no, emacs won't cut
it for me, nor mutt. Thank you very much for your suggestion.) So it is
really annoying for me to scroll through quoted text, page after page,
only to find that none of it got a response after all.

In short: I would really appreciate it if you could cut quoted text after
your last response.

Thanks,
Dscho

P.S.: It's this mailing list thing again. I would *love* to switch to a
mail client more to my liking, but the ones I like all won't respect white
space in patch files that are pasted verbatim into the mail body.