From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Fri, 13 May 2011 09:35:03 GMT
Subject: Re: [PATCH 4/8] revert: Separate cmdline argument handling from the functional code
Message-ID: <20110513093501.GE14272@ramkum.desktop.amazon.com>
In-Reply-To: <20110513090923.GB14272@ramkum.desktop.amazon.com>

Hi again,

Ramkumar Ramachandra writes:
> Jonathan Nieder writes:
> > Ramkumar Ramachandra wrote:
> > > --- a/builtin/revert.c
> > > +++ b/builtin/revert.c
> > > @@ -603,19 +603,12 @@ static int read_and_refresh_cache(struct replay_opts *opts)
> > >  	return 0;
> > >  }
> > >  
> > > -static int revert_or_cherry_pick(int argc, const char **argv,
> > > -				struct replay_opts *opts)
> > > +static int pick_commits(struct replay_opts *opts)
> > >  {
> > >  	struct rev_info revs;
> > >  	struct commit *commit;
> > > -	const char *me;
> > >  	int res;
> > >  
> > > -	git_config(git_default_config, NULL);
> > > -	me = (opts->action == REVERT ? "revert" : "cherry-pick");
> > > -	setenv(GIT_REFLOG_ACTION, me, 0);
> > > -	parse_args(argc, argv, opts);
> > > -
> > >  	if (opts->allow_ff) {
> > 
> > I don't see why the caller sets up GIT_REFLOG_ACTION, since the caller
> > is not making the commits.  Is there an example where it would use
> > something other than "cherry-pick" or "revert"?
> 
> Nice catch! Yes, GIT_REFLOG_ACTION should be in pick_commits.

Er, I mean in do_pick_commit.  Right?

-- Ram