Re: [GSoC][PATCH 1/1] sequencer: print an error message if append_todo_help() fails

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Alban,
>
> On Tue, 26 Jun 2018, Alban Gruin wrote:
>
>> This adds an error when append_todo_help() fails to write its message to
>> the todo file.
>> 
>> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
>
> ACK.
>
> We *may* want to fold that into the commit that adds `append_todo_help()`.

Absolutely.  This looks more like an "oops, I made a mess and here
is a fix on top", and even worse, it does not make an effort to help
readers where the mess was made (iow, which commit it goes on to
of); it is better to be squashed in.

I do not know offhand who Alban's mentors are, but one thing I think
is a good thing for them to teach is how to better organize the
changes with readers in mind.  The author of a patch series knows
his or her patches and how they relate to each other a lot better
than the readers of patches, who are reading not just his or her
patches but the ones from a lot wider set of contributors.  Even
though append-todo-help and edit-todo may have been developed as
separate steps in author's mind, it is criminal to send them as if
they are completely separate topics that can independently applied,
especially when one depends on the other.  It is a lot more helpful
to the readers if they were sent as a larger single series, because
doing so _will_ tell the readers which order the dependency goes.

> And, as I mentioned previously, I would love for that function to be used
> as an excuse to introduce the long-overdue `interactive-rebase.c`

I am not sure if I like this direction.  

As newbies are often very bad at coming up with APIs and naming
global functions, keeping everything as "static" inside a single
sequencer.c tends to avoid contaminating the global namespace.