From: Derrick Stolee <stolee@gmail.com>
Date: Wed, 31 Jan 2018 14:45:43 GMT
Subject: Re: [PATCH v2 09/27] transport: store protocol version
Message-ID: <ae3bd6ae-7335-9aca-0818-a90e202ff37a@gmail.com>
In-Reply-To: <20180125235838.138135-10-bmwill@google.com>

On 1/25/2018 6:58 PM, Brandon Williams wrote:
> +	switch (data->version) {
> +	case protocol_v1:
> +	case protocol_v0:
> +		refs = fetch_pack(&args, data->fd, data->conn,
> +				  refs_tmp ? refs_tmp : transport->remote_refs,
> +				  dest, to_fetch, nr_heads, &data->shallow,
> +				  &transport->pack_lockfile);
> +		break;
> +	case protocol_unknown_version:
> +		BUG("unknown protocol version");
> +	}

After seeing this pattern a few times, I think it would be good to 
convert it to a macro that calls a statement for protocol_v1/v0 (and 
later calls a different one for protocol_v2). It would at minimum reduce 
the code clones surrounding this handling of unknown_version, and we 
could have one place that is clear this BUG() is due to an unexpected 
response from discover_version().