Re: [PATCH 2/2] pull: support rebased upstream + fetch + pull --rebase

[Junio C Hamano <gitster@pobox.com>]

Santi Béjar <santi@agolina.net> writes:

>  	reflist="$(get_remote_merge_branch "$@" 2>/dev/null)" &&
> -	oldremoteref="$(git rev-parse -q --verify \
> -		"$reflist")"
> +	num=0 &&
> +	while oldremoteref="$(git rev-parse -q --verify "$reflist@{$num}")"

Applying @{nth} reflog notation to something that identifies itself as a
"list" made me go "Huh?".  Why is this variable called refLIST?  Shouldn't
it be simply called something like "remoteref" or even "ref"?

> +	do
> +		test $oldremoteref = $(git merge-base $oldremoteref $curr_branch) &&
> +		break
> +		num=$((num+1))

I think we always write "num=$(( $num + 1 ))" for portability; notice the
lack of $ in your version.

> +	done

Does this loop ever give up?  Should it?

What happens in the subsequent code outside of the patch context, when
this loop does not find any suitable "old" value?

Other than that, looking good.

Thanks.