Re: [PATCH 31/31] rebase -i: remove unnecessary state rebase-root

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 28 Dec 2010, Thomas Rast wrote:

> Martin von Zweigbergk wrote:
> > @@ -168,11 +168,6 @@ pick_one () {
> >  	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
> >  	test -d "$REWRITTEN" &&
> >  		pick_one_preserving_merges "$@" && return
> > -	if test -n "$rebase_root"
> > -	then
> > -		output git cherry-pick "$@"
> > -		return
> > -	fi
> >  	output git cherry-pick $ff "$@"
> >  }
> [...]
> > While factoring out the state writing code a few patches back, I went
> > through each of the pieces of state that was written. I was a bit
> > hesitant to include this patch since I'm not quite sure why the code
> > was introduced, but I thought I would include it anyway to hear what
> > you have to say.
> > 
> > There used to be bug when using --ff when rebasing a root commit. This
> > was fixed in 6355e50 (builtin/revert.c: don't dereference a NULL
> > pointer, 2010-09-27). Could this have been the reason for the check?
> > Thomas, do you remember?
> 
> I think this just ended up being such a strange test because of the
> following hunk in 8e75abf (rebase -i: use new --ff cherry-pick option,
> 2010-03-06):
> 
> @@ -232,16 +232,7 @@ pick_one () {
>                 output git cherry-pick "$@"
>                 return
>         fi
> -       parent_sha1=$(git rev-parse --verify $sha1^) ||
> -               die "Could not get the parent of $sha1"
> -       current_sha1=$(git rev-parse --verify HEAD)
> -       if test -z "$no_ff" && test "$current_sha1" = "$parent_sha1"
> -       then
> -               output git reset --hard $sha1
> -               output warn Fast-forward to $(git rev-parse --short $sha1)
> -       else
> -               output git cherry-pick "$@"
> -       fi
> +       output git cherry-pick $ff "$@"
>  }
> -- 

Yes, I saw that one as well, so it would probably have made more sense
to ask Christian instead (the author of 8e75abf). So do you remember,
Christian?

Anyway, thanks for your input, Thomas. That makes me feel a little
less worried about this patch.


/Martin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]