Subject: Re: [PATCH 13/17] revert: Introduce a layer of indirection over pick_commits

From: Jonathan Nieder <[email-masked]>
Date: Tue, 12 Jul 2011 15:03:24 -0500

Ramkumar Ramachandra wrote:

> Write a new function called process_subcommand 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.  This will be useful when
> subcommands like "--reset" and "--continue" are introduced later in
> the series.

Could you give an example of how this function is used?  The name
"process_foo" does not mean much more to me than "do_it" --- i.e., it
does not tell what the function actually does.

Looking at this code, it does not just prepare a todo_list (so
fill_todo_list would not be a good name for it); it actually calls
pick_commits.  This is the main (only?) entry point to the cherry-pick
machinery, so it's worthwhile to give it a good name.  How about
pick_commits?

Meanwhile I would prefer to have multiple entry points to the
cherry-pick machinery, which would avoid this question altogether,
though I imagine you have some reason not to do that.

The mechanics of the patch (splitting out a separate function for
the actual "for each commit" loop) look like a good idea.