Subject: Re: [PATCH v3 2/3] transfer.advertiseObjectInfo: add object-info config

From: Taylor Blau <[email-masked]>
Date: Tue, 29 Mar 2022 19:09:49 -0400

On Tue, Mar 29, 2022 at 03:48:28PM -0700, Calvin Wan wrote:
> > It strikes me somewhat odd that this is the _first_ such capability
> > that needs a variable here to control if it is advertised or not
> > depending on the configuration file settings.
>
> > In other words, "Why doesn't transfer.advertiseSID also have a
> > similar thing in this file?" should be the first question that comes
> > to any reviewers' mind.
>
> It doesn't exist in protocol-caps.c, but instead in serve.c. Would be
> a good discussion for whether the advertise function should be serve.c
> or protocol-caps.c

Most of the others are defined statically within serve.c, but there are
a couple of exceptions:

  - `ls_refs_advertise()` is defined in ls-refs.c, and
  - `upload_pack_advertise()` is defined in upload-pack.c.

Even though cap_object_info() is defined in protocol-caps.c, I think we
should probably keep the definition of `object_info_advertise()` local
to serve.c.

If there were a object-info.c, it would make sense to put it there, but
since there isn't, it makes more sense to keep it local to serve.c,
which avoids us having to declare the function in a header file.

Thanks,
Taylor