Re: [PATCH v3 05/10] upload-pack, receive-pack: introduce protocol version 1

On 10/10, Jonathan Tan wrote:

On Tue,  3 Oct 2017 13:15:02 -0700
Brandon Williams [off-list ref] wrote:

quoted

+	switch (determine_protocol_version_server()) {
+	case protocol_v1:
+		if (advertise_refs || !stateless_rpc)
+			packet_write_fmt(1, "version 1\n");
+		/*
+		 * v1 is just the original protocol with a version string,
+		 * so just fall through after writing the version string.
+		 */

Peff sent out at least one patch [1] that reformats fallthrough comments
to be understood by GCC. It's probably worth doing here too.

In this case, I would do the 2-comment system that Peff suggested:

	/*
	 * v1 is just the original protocol with a version string,
	 * so just fall through after writing the version string.
	 */
	if (advertise_refs || !stateless_rpc)
		packet_write_fmt(1, "version 1\n");
	/* fallthrough */

(I put the first comment before the code, so it doesn't look so weird.)

Sounds good.

[1] https://public-inbox.org/git/20170921062541.ew67gyvrmb2ot4sf@sigill.intra.peff.net/

quoted

+	case protocol_v0:
+		break;
+	default:

I'm also going to change this to from default to
'protocol_unknown_version' that way we get a compiler error instead of a
run-time BUG when introducing a new protocol version number.

quoted

+		BUG("unknown protocol version");
+	}

-- 
Brandon Williams