Re: [PATCH v3 16/35] transport: convert transport_get_remote_refs to take a list of ref patterns

[Brandon Williams]

On 02/22, Jonathan Tan wrote:
> On Thu, 22 Feb 2018 10:26:47 -0800
> Brandon Williams  wrote:
> 
> > On 02/21, Jonathan Tan wrote:
> > > On Tue,  6 Feb 2018 17:12:53 -0800
> > > Brandon Williams  wrote:
> > > 
> > > > -const struct ref *transport_get_remote_refs(struct transport 
> > > > *transport)
> > > > +const struct ref *transport_get_remote_refs(struct transport 
> > > > *transport,
> > > > +   const struct argv_array 
> > > > *ref_patterns)
> > > >  {
> > > > if (!transport->got_remote_refs) {
> > > > -   transport->remote_refs = 
> > > > transport->vtable->get_refs_list(transport, 0, NULL);
> > > > +   transport->remote_refs =
> > > > +   transport->vtable->get_refs_list(transport, 0,
> > > > +ref_patterns);
> > > > transport->got_remote_refs = 1;
> > > > }
> > > 
> > > Should we do our own client-side filtering if the server side cannot do
> > > it for us (because it doesn't support protocol v2)? Either way, this
> > > decision should be mentioned in the commit message.
> > 
> > If someone wants to add this in the future they can, but that is outside
> > the scope of this series.
> 
> In that case, also document that this function is allowed to return refs
> that do not match the ref patterns.
> 
> Unlike in patch 15 (which deals with the interface between the transport
> code and transport vtables, which can be changed as long as the
> transport code is aware of it, as I wrote in [1]), this may result in
> user-visible differences depending on which protocol is used. But after
> more thinking, I don't think we're in a situation yet where having extra
> refs shown/written are harmful, and if it comes to that, we can tighten
> this code later without backwards incompatibility. So, OK, this is fine.
> 
> [1] 
> https://public-inbox.org/git/20180221145639.c6cf2409ce2120109bdd1...@google.com/

I'll add the documentation.

-- 
Brandon Williams

Re: [PATCH v3 16/35] transport: convert transport_get_remote_refs to take a list of ref patterns

[Jonathan Tan]

On Thu, 22 Feb 2018 10:26:47 -0800
Brandon Williams  wrote:

> On 02/21, Jonathan Tan wrote:
> > On Tue,  6 Feb 2018 17:12:53 -0800
> > Brandon Williams  wrote:
> > 
> > > -const struct ref *transport_get_remote_refs(struct transport *transport)
> > > +const struct ref *transport_get_remote_refs(struct transport *transport,
> > > + const struct argv_array 
> > > *ref_patterns)
> > >  {
> > >   if (!transport->got_remote_refs) {
> > > - transport->remote_refs = 
> > > transport->vtable->get_refs_list(transport, 0, NULL);
> > > + transport->remote_refs =
> > > + transport->vtable->get_refs_list(transport, 0,
> > > +  ref_patterns);
> > >   transport->got_remote_refs = 1;
> > >   }
> > 
> > Should we do our own client-side filtering if the server side cannot do
> > it for us (because it doesn't support protocol v2)? Either way, this
> > decision should be mentioned in the commit message.
> 
> If someone wants to add this in the future they can, but that is outside
> the scope of this series.

In that case, also document that this function is allowed to return refs
that do not match the ref patterns.

Unlike in patch 15 (which deals with the interface between the transport
code and transport vtables, which can be changed as long as the
transport code is aware of it, as I wrote in [1]), this may result in
user-visible differences depending on which protocol is used. But after
more thinking, I don't think we're in a situation yet where having extra
refs shown/written are harmful, and if it comes to that, we can tighten
this code later without backwards incompatibility. So, OK, this is fine.

[1] 
https://public-inbox.org/git/20180221145639.c6cf2409ce2120109bdd1...@google.com/

Re: [PATCH v3 16/35] transport: convert transport_get_remote_refs to take a list of ref patterns

[Brandon Williams]

On 02/21, Jonathan Tan wrote:
> On Tue,  6 Feb 2018 17:12:53 -0800
> Brandon Williams  wrote:
> 
> > -const struct ref *transport_get_remote_refs(struct transport *transport)
> > +const struct ref *transport_get_remote_refs(struct transport *transport,
> > +   const struct argv_array 
> > *ref_patterns)
> >  {
> > if (!transport->got_remote_refs) {
> > -   transport->remote_refs = 
> > transport->vtable->get_refs_list(transport, 0, NULL);
> > +   transport->remote_refs =
> > +   transport->vtable->get_refs_list(transport, 0,
> > +ref_patterns);
> > transport->got_remote_refs = 1;
> > }
> 
> Should we do our own client-side filtering if the server side cannot do
> it for us (because it doesn't support protocol v2)? Either way, this
> decision should be mentioned in the commit message.

If someone wants to add this in the future they can, but that is outside
the scope of this series.

-- 
Brandon Williams

Re: [PATCH v3 16/35] transport: convert transport_get_remote_refs to take a list of ref patterns

[Jonathan Tan]

On Tue,  6 Feb 2018 17:12:53 -0800
Brandon Williams  wrote:

> -const struct ref *transport_get_remote_refs(struct transport *transport)
> +const struct ref *transport_get_remote_refs(struct transport *transport,
> + const struct argv_array 
> *ref_patterns)
>  {
>   if (!transport->got_remote_refs) {
> - transport->remote_refs = 
> transport->vtable->get_refs_list(transport, 0, NULL);
> + transport->remote_refs =
> + transport->vtable->get_refs_list(transport, 0,
> +  ref_patterns);
>   transport->got_remote_refs = 1;
>   }

Should we do our own client-side filtering if the server side cannot do
it for us (because it doesn't support protocol v2)? Either way, this
decision should be mentioned in the commit message.