Re: [PATCH v2 2/3] promisor-remote: allow a server to advertise more fields

> > I am not sure I understand what you mean. This promisor.sendFields
> > config variable is for the server side which advertises remotes. The
> > server advertises its remotes (if it wants to) before receiving
> > information from the client, so it cannot know what the client
> > accepts.
>
> In the current form you need to reflow this whole paragraph every time a
> new field is supported, and it's easy to miss the exact supported
> fields. So my idea was to maybe move the supported fields into a
> bulleted list. E.g.:
>
>     promisor.sendFields::
>          A comma or space separated list of additional remote related
>          fields that a server will send while advertising its promisor
>          remotes using the "promisor-remote" capability, see
>          linkgit:gitprotocol-v2[5]. The following fields are supported:
>     +
>     * "partialCloneFilter": contains the partial clone filter used for
>       the remote.
>     * "token": contains the authentication token for the remote.

> > > Furthermore, should we maybe refactor this to match the restrictive
> > > design where valid fields are explicitly specified? In other words,
> > > should we have separate config keys for each of the accepted fields now?
> >
> > Maybe I don't understand what you mean with "accepted fields".
>
> I think I had a misunderstanding on my side. I didn't get that this is
> only configuring field _names_ that we'll end up sending to the remote
> side. So I thought that the user is expected to configure name-value
> pairs here that are then sent to the client, not only the name.
>
> I guess this is mostly because the config documentation talks about
> "fields", but that term is used elsewhere to indicate a name-value pair.

> > > Does
> > > it mean that this promisor remote should only be used in case we do have
> > > the exact same filter passed to git-clone(1)?
> >
> > It's up to the client to decide, but yeah it will likely work better
> > if the same filter is used. It should still work if a different filter
> > is used though. In case the promisor remote doesn't have an object,
> > there should be a fallback to ask the main server for that object.
> >
> > Also the filter mechanism already exists for a long time and this
> > series doesn't change how it works. It's already possible to have
> > different repos using the same promisor remote with different filters.
> > So documentation about what happens when they do that should not be
> > specific to this patch series.
>
> That's fair enough, but spelling this out somewhere and drawing the
> bigger picture helps the reviewer understand the vision that you've got
> here.