From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 09 Dec 2011 19:02:36 GMT
Subject: Re: [PATCH 9/9] revert: simplify communicating command-line arguments
Message-ID: <20111209190236.GA20913@elie.hsd1.il.comcast.net>
In-Reply-To: <1323445326-24637-10-git-send-email-artagnon@gmail.com>

Hey,

Ramkumar Ramachandra wrote:

> From: Jonathan Nieder <jrnieder@gmail.com>
[...]
> Future callers as other commands are built in (am, rebase, sequencer)
> may find it easier to pass rev-list options to this machinery in
> already-parsed form.  So, teach cmd_cherry_pick and cmd_revert to
> parse the rev-list arguments in advance and pass the commit set to
> pick_revisions() as a "struct rev_info".
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

This sign-off chain suggests that the life of the patch happened in
three stages:

 - first, you wrote the patch or some component of it and passed it to
   me.
 - then, I accepted it, tweaked it so much that I felt the need to claim
   authorship and save you from blame (hence the "From:" field), and
   passed it on to Junio.
 - finally, Junio applied it to some tree.

and that you are sending out a copy of the patch Junio applied for
additional comments.

But in fact, this started with a patch from me (I don't rememember
whether it was signed off, but that doesn't matter --- I happily
retroactively sign off on it) and now you are sending it to the list
and Junio for comments.  So I would think it should simply say

	Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
	Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>

and Junio can add his sign-off below that when applying some version
to his tree.  Bonus points if you mention what tweaks you made when
you are not just passing it on.

[...]
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -414,4 +414,15 @@ test_expect_success 'mixed pick and revert instructions' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'empty commit set' '
> +	pristine_detach initial &&
> +	test_expect_code 128 git cherry-pick base..base
> +'
> +
> +test_expect_success 'commit set passed through --all' '
> +	pristine_detach initial &&
> +	test_expect_code 1 git cherry-pick --all &&
> +	git cherry-pick --continue
> +'

What does this test mean?  "git cherry-pick --all" should report a
spurious conflict, and then --continue will clean it up automagically
and all will be well?