Subject: Re: [PATCH 2/4] pull: use --quiet rather than 2>/dev/null

From: Jonathan Nieder <[email-masked]>
Date: Sat, 20 Mar 2010 07:35:20 -0500

Benjamin C Meyer wrote:

> -	for reflog in $(git rev-list -g $remoteref 2>/dev/null)
> +	for reflog in $(git rev-list --quiet -g $remoteref)

Are you sure?  My local copy of git-rev-list.1 says

    --quiet
         Don’t print anything to standard output. This form is primarily meant
         to allow the caller to test the exit status to see if a range of
         objects is fully connected (or not). It is faster than redirecting
         stdout to /dev/null as the output does not have to be formatted.

A similar question applies to the other patches in this series: are
you sure they suppress the right output?  (I haven’t checked, just
asking.)

Aside: that for loop looks like it could be improved.  Maybe it is worth
factoring this into a separate function, something like:

old_upstream() {
	remoteref=$1 &&
	curr_branch=$2 &&

	oldremoteref="$(git rev-parse -q --verify "$remoteref")" &&
	{ git rev-list -g "$remoteref" 2>/dev/null || return $?; } |
	while read reflog
	do
		if test -z "$(git rev-list "$curr_branch".."$reflog" | head -n 1)"
		then
			printf "%s\n" "$reflog"
			return 0
		fi
	done &&
	printf "%s\n" "$oldremoteref"
}

In other words, we can avoid walking the whole reflog before starting
to look for an ancestor for the current branch.

Jonathan