On Mon, May 18, 2020 at 5:18 AM Denton Liu <liu.denton@xxxxxxxxx> wrote: > On Wed, May 13, 2020 at 07:14:35PM -0400, Eric Sunshine wrote: > > > + case protocol_v1: > > > die_if_server_options(transport); > > > + /* fallthrough */ > > > + case protocol_v2: > > > refs = fetch_pack(&args, data->fd, > > > refs_tmp ? refs_tmp : transport->remote_refs, > > > to_fetch, nr_heads, &data->shallow, > > > > I can't say that I'm a fan of this change. While it may make it clear > > that the two calls are identical, it makes it more difficult to reason > > about the distinct v0, v1, and v2 cases. > > Actually, thinking about it some more, do you think it would make more > sense to just pull the fetch_pack() call out of the switch statement > entirely? We could entirely eliminate the fallthrough if we do this. Yes, I think that works better and is cleaner and easier to understand than the "fallthrough" in v1.
