Re: [PATCH 4/5] sequencer: handle single commit pick separately

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 6, 2011, 00:35 UTC
Message-ID: <20111106003519.GF27272@elie.hsd1.il.comcast.net>
In-Reply-To: <1320510586-3940-5-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

> Don't write a '.git/sequencer/todo', as CHERRY_PICK_HEAD already
> contains this information.  However, '.git/sequencer/opts' and
> '.git/sequencer/head' are required to support '--reset' and
> '--continue' operations.

This is meant as a signal to later "git cherry-pick" commands that it is okay to forget about the cherry-pick, right? How is the reader supposed to know that? Say so!

By the way, it's not clear to me yet whether the resulting UI would be more pleasant or not. What is the expected calling sequence? Any odd corners of behavior changing? What happens if I do

	git cherry-pick foo; # conflicts!
	git cherry-pick bar; # just ignore them

or

	git cherry-pick foo; # conflicts!  but resolved in index by rerere
	git checkout something-else

Is there any potential downside to the change?

[...]

Show 11 quoted lines

> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -746,6 +746,15 @@ static int pick_commits(struct replay_insn_list *todo_list,
>  				opts->record_origin || opts->edit));
>  	read_and_refresh_cache(opts);
>  
> +	/*
> +	 * Backward compatibility hack: when only a single commit is
> +	 * picked, don't save_todo(), because CHERRY_PICK_HEAD will
> +	 * contain this information anyway.
> +	 */

How does saving disk space by avoiding saving redundant information affect backward compatibility? I'm not sure what this comment is trying to say.

Show 5 quoted lines

> +	if (opts->subcommand == REPLAY_NONE &&
> +		todo_list->next == NULL && todo_list->action == REPLAY_PICK)
> +		return do_pick_commit(todo_list->operand, REPLAY_PICK, opts);
> +
>  	for (cur = todo_list; cur; cur = cur->next) {