Re: [PATCH v2 2/2] remote.c: fix handling of %(push:remoteref)

From: Damien Robert <damien.olivier.robert@gmail.com>
Date: Mar 3, 2020, 16:29 UTC
Message-ID: <20200303162906.xadbaeaq4nurqsem@feanor>
In-Reply-To: <20200303161223.1870298-3-damien.olivier.robert+git@gmail.com>

I have some remarks/questions:

From Damien Robert, Tue 03 Mar 2020 at 17:12:23 (+0100) :

Show 7 quoted lines

> +	if (remote->push.nr) {
> +		char *dst;
> +		dst = apply_refspecs(&remote->push, branch->refname);
> +		if (!dst)
> +			return NULL;
> +		return dst;
> +	}

Should I simply `return apply_refspecs(&remote->push, branch->refname);` here, or is it a good form to always check for a NULL return value even if we do nothing with it?

> +	case PUSH_DEFAULT_MATCHING:
> +	case PUSH_DEFAULT_CURRENT:
> +		return branch->refname;

Here I follow the logic of branch_get_push1, but the case of push.default=matching is not quite correct, because we never check that we have a matching remote branch. On the other hand we cannot check this until we contact the remote, so I don't know how we could get around that.

-- 
Damien Robert
http://www.normalesup.org/~robert/pro