Re: [PATCH 06/17] revert: Eliminate global "commit" variable

[Jonathan Nieder <jrnieder@gmail.com>]

Ramkumar Ramachandra wrote:

> Since we want to develop the functionality to either pick or revert
> individual commits atomically later in the series, make "commit" a
> variable to be passed around explicitly as an argument for clarity.

The above explanation is not so clear to me, but the patch looks good.
Isn't the idea something like

	commit = grab_a_nice_commit();
	res = do_pick_commit();

being just an unpleasant API relative to

	res = do_pick_commit(grab_a_nice_commit());

because in the latter it is more obvious which commit is being
cherry-picked?  Likewise with the functions it calls.

Or perhaps the idea is that eventually we will want to expose something
like do_pick_commit to other translation units, but a static variable
like "commit" would not be appropriate for exposing.  Or that we save
a word of global memory.  Or that this way if do_pick_commit or a
function it calls ever ends up recursing by mistake it won't get
broken.  Or that we can use multiple threads some day.  Or...

Oh, the uncertainty! :)  It is not clear to me what any of the above
have to do with wanting the functionality to replay an individual
commit atomically.  By the way, what does pickiing or reverting a
commit atomically mean, and how is it different from ordinary
cherry-picks?