Re: [PATCH v2 00/11] sha1_name: improvements

From: Junio C Hamano <gitster@pobox.com>
Date: May 8, 2013, 05:56 UTC
Message-ID: <7v1u9igglk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAMP44s3-vVUB4VnZP4uBMLAbviV+BMTqDcbO_TxkX+5RE6cnSg@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 17 quoted lines

> When merging this series to the @ shortcut one, there will be
> conflicts, this is how I propose fixing them:
>
>                 return len; /* syntax Ok, not enough switches */
> -       if (0 < len && len == namelen)
> +       if (len > 0 && len == namelen)
>                 return len; /* consumed all */
> -       else if (0 < len)
> ...
> ++      else if (len > 0)
>  +              return reinterpret(name, namelen, len, buf);
>
> - check "@" new-two
> - check "@@{u}" upstream-two
> ...
> ++check "@" ref refs/heads/new-branch
> ++check "@@{u}" ref refs/heads/upstream-branch

The resolution for the tests wrapper that acquired an extra parameter matches what I did locally. Thanks for a merge sanity check.

I didn't see any conflicts on the sha1_name.c side, but I applied the Yoda thing slightly differently to result in a slightly more streamlined codeflow:

	if (!len) {
		return len;
	} else if (len > 0) {
		if (len == namelen)
                	return len;
		else
			return reinterpret(...);
	}

which I think shows the choices better.

Although I haven't looked at the largest one (10/11) carefully, everything else looked quite straightforward and readable.

I am not very happy about how $<n> parameters are quoted in t1508, but that suboptimal quoting were there before this series, and I'd consider it outside of the scope for now.

Will queue.  Thanks.