Re: [PATCH v4 2/3] connect, transport: add no-op arg for future patch
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jan 26, 2021, 18:16 UTC
- Message-ID
- <20210126181606.2335396-1-jonathantanmy@google.com>
- In-Reply-To
- <YAnqQZaVByUYYRNr@coredump.intra.peff.net>
Show 22 quoted lines
> On Tue, Dec 22, 2020 at 01:54:19PM -0800, Jonathan Tan wrote:
>
> > In a future patch we plan to return the name of an unborn current branch
> > from deep in the callchain to a caller via a new pointer parameter that
> > points at a variable in the caller when the caller calls
> > get_remote_refs() and transport_get_remote_refs(). Add the parameter to
> > functions involved in the callchain, but no caller passes an actual
> > argument yet in this step. Thus, the future patch only needs to concern
> > itself with new logic.
>
> OK. Since the call stack is so deep, it's nice to get all of this diff
> noise out of the way of the third patch.
>
> It does make me wonder if we should be passing a struct like:
>
> struct transport_fetch_options {
> struct strvec ref_prefixes;
> char **unborn_head;
> }
> #define TRANSPORT_FETCH_OPTIONS_INIT = { STRVEC_INIT }
>
> which would solve this problem once for any future options.That's a good idea, and I've switched patch 2 to doing this. It also makes it easier to explain (no "unborn_head" dummy variable that does nothing, since I can just introduce "unborn_head" in patch 3).
Show 16 quoted lines
> > @@ -455,7 +455,8 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, > > struct ref **list, int for_push, > > const struct strvec *ref_prefixes, > > const struct string_list *server_options, > > - int stateless_rpc) > > + int stateless_rpc, > > + char **unborn_head_target) > > Is a single string enough? The way the protocol is defined, I think the > server is free to tell us about other unborn symrefs, too (but of course > our implementation does not). And I'm not sure what we'd do with such > values (in a "--mirror" clone, I guess we could make local copies of > them). > > Should we be prepared for that at the transport layer, or is it > over-engineering?
With the struct, I think we're prepared for it.