Re: [PATCH v2 4/9] rebase -i: refactor transform_todo_ids

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> On Sun, 3 Dec 2017, Liam Beguin wrote:
>>
>>> The transform_todo_ids function is a little hard to read. Lets try
>>> to make it easier by using more of the strbuf API. Also, since we'll
>>> soon be adding command abbreviations, let's rename the function so
>>> it's name reflects that change.
>>
>> I am not really a fan of the new name, and would prefer the old one, but
>> that's only a nit, not a reason to reject the patch.
> 
> FWIW, I do think the new name goes backwards.  The command uses to
> remember what operations are to be carried out in which order using
> a thing, and the name of that thing "todo list".  We also called it
> the "instruction sheet", and "insn" was a good term to call one item
> on that sheet among other items.
> 

> But recent commits in the area are pushing us to call it "todo list"
> consistently.  An element in that list should be called "todo".
> 
> A "todo" consists of two parts, "what operation is done" part and
> "using what commit object" part.  The original implementation of
> this function affected only the latter part, and in that light, the
> original name transform_todo_ids() is understandable.  Now you are
> planning to make it modify both parts, not just "ids", so it is
> understandable that you would want to rename it.  But I do not think
> "insn" matches the recent trend.  It even risks misunderstanding
> (i.e. xfrm_todo_ids() is about modifying "using what commit" part,
> so perhaps xfrm_todo_insns() is about modifying "what operation is
> done" part---but that is different from what you want to do, which
> is to update _both_ halves).
>