Re: [PATCH 11/14] revert: Introduce a layer of indirection over pick_commits

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 6, 2011, 11:24 UTC
Message-ID: <CALkWK0=0vBUG_UOLWt+SFMctfW1__OOG-=BAY4WwMGM5OfDj4A@mail.gmail.com>
In-Reply-To: <20110706103734.GL15682@elie>

Hi again,

Jonathan Nieder writes:

Show 7 quoted lines

> Ramkumar Ramachandra wrote:
>> Write a new function called process_continuation to prepare a
>> todo_list to call pick_commits with; the job of pick_commits is
>> simplified into performing the tasks listed in todo_list.
>
> Why is it called process_continuation?  What is its responsibility?
> When would I call it?

I wanted a general name for the features I'm writing (--reset, --continue): I want to call these "continuation features". I personally like the term, because it reminds me of the "call/cc" in Scheme.

Show 8 quoted lines

>> +     /*
>> +      * Decide what to do depending on the arguments; a fresh
>> +      * cherry-pick should be handled differently from an existing
>> +      * one that is being continued
>> +      */
>> +     res = process_continuation(&opts);
>
> Is this the new sole entry point to the cherry-pick/revert machinery?

Yes.

Show 5 quoted lines

> In that case, I'd be mildly tempted to call it something crazy like
> start_or_continue_replay(), and even more tempted to split it into
> separate entry points for new_replay(), continue_replay(),
> abort_replay(), and remove_replay_state() (but please don't trust me
> about the names; this is just to get the idea across).

Why? Is introducing new terminology so bad? Should I explain what I mean by "continuation" in the commit message/ a comment?

-- Ram