Subject: Re: [PATCH RFC v2 05/19] rebase -i: Implement reword in terms of do_pick

From: Fabian Ruch <[email-masked]>
Date: Mon, 04 Aug 2014 17:45:51 +0200

Hi Matthieu,

thanks for taking a look at this patch series. I might have caused some
confusion by starting with version v1 again after removing the RFC tag.
The current reroll[1] is tagged with PATCH v1, not PATCH RFC v2.

I'm sorry if this is the reason why your reply appears on this sub-thread.

Your concerns below are of course noted.

   Fabian

[1] http://article.gmane.org/gmane.comp.version-control.git/254361

Matthieu Moy writes:
> Fabian Ruch <[email-masked]> writes:
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -555,20 +555,7 @@ do_next () {
>>  		comment_for_reflog reword
>>  
>>  		mark_action_done
>> -		do_pick $sha1 "$rest"
>> -		# TODO: Work around the fact that git-commit lets us
>> -		# disable either both the pre-commit and the commit-msg
>> -		# hook or none. Disable the pre-commit hook because the
>> -		# tree is left unchanged but run the commit-msg hook
>> -		# from here because the log message is altered.
>> -		git commit --allow-empty --amend --no-post-rewrite -n ${gpg_sign_opt:+"$gpg_sign_opt"} &&
>> -			if test -x "$GIT_DIR"/hooks/commit-msg
>> -			then
>> -				"$GIT_DIR"/hooks/commit-msg "$GIT_DIR"/COMMIT_EDITMSG
>> -			fi || {
>> -				warn "Could not amend commit after successfully picking $sha1... $rest"
>> -				exit_with_patch $sha1 1
>> -			}
>> +		do_pick --edit $sha1 "$rest"
> 
> I would have found this easier to review if squashed into the previous
> patch. My reaction reading the previous patch was "Uh, why duplicate
> code?", and reading this one "Ah, that's OK". A single patch doing both
> would have avoided the confusion.