mbox series

[0/5] serve: add "configure" callback

Message ID cover-0.5-00000000000-20210616T141332Z-avarab@gmail.com (mailing list archive)
Headers show
Series serve: add "configure" callback | expand

Message

Ævar Arnfjörð Bjarmason June 16, 2021, 2:16 p.m. UTC
This is a refactoring of what a callback in serve.c needs to do to
aquire config. Currently two of them want that, and grab it in ad-hoc
ways themselves, now they can insted configure a "configure" callback
along with the existing "advertise" and "command", by the time they're
called their config will already be read with their callback.

I split this prep work off from an upcoming series where I wanted to
add a new capability, but I think this stands nicely on its own, and
simplifies the existing code.

The line count increase is mostly converting things to designated
initializers.

Ævar Arnfjörð Bjarmason (5):
  serve: mark has_capability() as static
  transport: rename "fetch" in transport_vtable to "fetch_refs"
  transport: use designated initializers
  serve: use designated initializers
  serve: add support for a git_config() callback

 ls-refs.c            | 55 +++++++++++---------------------
 ls-refs.h            |  1 +
 serve.c              | 76 ++++++++++++++++++++++++++++++++++++++------
 serve.h              |  3 --
 transport-helper.c   | 18 +++++------
 transport-internal.h |  2 +-
 transport.c          | 32 ++++++++-----------
 7 files changed, 108 insertions(+), 79 deletions(-)

Comments

Jeff King June 16, 2021, 4:23 p.m. UTC | #1
On Wed, Jun 16, 2021 at 04:16:16PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This is a refactoring of what a callback in serve.c needs to do to
> aquire config. Currently two of them want that, and grab it in ad-hoc
> ways themselves, now they can insted configure a "configure" callback
> along with the existing "advertise" and "command", by the time they're
> called their config will already be read with their callback.
> 
> I split this prep work off from an upcoming series where I wanted to
> add a new capability, but I think this stands nicely on its own, and
> simplifies the existing code.

The first four seem like obviously-good cleanups. I'm not so sure
about the last one, though. I left some comments there.

-Peff
Junio C Hamano June 17, 2021, 12:49 a.m. UTC | #2
Jeff King <peff@peff.net> writes:

> On Wed, Jun 16, 2021 at 04:16:16PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> This is a refactoring of what a callback in serve.c needs to do to
>> aquire config. Currently two of them want that, and grab it in ad-hoc
>> ways themselves, now they can insted configure a "configure" callback
>> along with the existing "advertise" and "command", by the time they're
>> called their config will already be read with their callback.
>> 
>> I split this prep work off from an upcoming series where I wanted to
>> add a new capability, but I think this stands nicely on its own, and
>> simplifies the existing code.
>
> The first four seem like obviously-good cleanups. I'm not so sure
> about the last one, though. I left some comments there.

I had the same impression.  Thanks, both.