Re: [PATCH v2 7/8] serve: add support for a "startup" git_config() callback

[Han-Wen Nienhuys <hanwen@google.com>]

On Mon, Jun 28, 2021 at 9:19 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> So let's support this common pattern as a "startup_config" callback,
> making use of our recently added "call_{advertise,command}()"

I think this is an improvement over ensure_config_read(), but I don't
understand the whole picture. If you want to control configuration and
be sure it is set early, isn't it more natural to put this in struct
serve_options? With your patch, there are still two routes to
injecting configuration: 'struct serve_options' and the git config.

IMO the former is more principled, and the control flow (serve_options
is an argument to serve()) ensures that the options are read only
once.

> +       /*
> +        * A git_config() callback that'll be called only once for the
> +        * lifetime of the process, possibly over many different

Putting it in a struct supplied by the caller provides more
flexibility for testing: you could test multiple behaviors from a
single process, by calling serve multiple times supplying different
arguments.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado