On Wed, May 7, 2025 at 2:27 PM Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>
> Christian Couder <christian.couder@xxxxxxxxx> writes:
>
> [snip]
>
> > +/*
> > + * Linked list for promisor remotes involved in the "promisor-remote"
> > + * protocol capability.
> > + *
> > + * 'fields' contains a defined set of field name/value pairs for
> > + * each promisor remote. Field names are stored in the 'string'
> > + * member, and values in the 'util' member.
> > + *
> > + * Currently supported field names:
> > + * - "name": The name of the promisor remote.
> > + * - "url": The URL of the promisor remote.
> > + *
> > + * Except for "name", each "<field_name>/<field_value>" pair should
> > + * correspond to a "remote.<name>.<field_name>" config variable set to
> > + * <field_value> where "<name>" is a promisor remote name.
> > + *
> > + * 'fields' should not be sorted, as we will rely on the order we put
> > + * things into it. So, for example, 'string_list_append()' should be
> > + * used instead of 'string_list_insert()'.
> > + */
> >
>
> Dumb question: As I read through the patch, I realized we really care
> about the order of the fields and it is mentioned here too. Why is the
> order important? Shouldn't the client be satisfied as long as the
> required fields are present? Or is this merely an implementation issue
> where we simply parse the information in a specific order?
We used to only care about the order for the first 2 mandatory fields
("name" and "url"), but this is relaxed in the v3.
Anyway I think it's better if the 2 mandatory fields appear first as
in many cases it might theoretically enable the client to reject an
advertised remote as soon as it has parsed the first 2 fields. So the
client could stop parsing the rest of the remote information in some
cases which might reduce its workload a bit and speed things up.
Yeah, I don't think it matters now, and we can take care of this if it
ever matters in the future. But if it ever matters, we might be happy
to have specified for a long time that "name" and "url" should appear
first.
