From: Ramkumar Ramachandra Date: Wed, 06 Jul 2011 11:24:12 GMT Subject: Re: [PATCH 11/14] revert: Introduce a layer of indirection over pick_commits Message-ID: In-Reply-To: <20110706103734.GL15682@elie> Hi again, Jonathan Nieder writes: > 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. >> +     /* >> +      * 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. > 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