On Tue, Jun 01, 2021 at 02:34:17PM -0700, Jonathan Tan wrote:
>
> Instead of using global variables to store promisor remote information,
> store this config in struct repository instead, and add
> repository-agnostic non-static functions corresponding to the existing
> non-static functions that only work on the_repository.
Nice!
> diff --git a/promisor-remote.c b/promisor-remote.c
> index bfe8eee5f2..5819d2cf28 100644
> --- a/promisor-remote.c
> +++ b/promisor-remote.c
> @@ -5,7 +5,11 @@
> #include "transport.h"
> #include "strvec.h"
>
> -static char *repository_format_partial_clone;
> +struct promisor_remote_config {
> + char *repository_format_partial_clone;
> + struct promisor_remote *promisors;
> + struct promisor_remote **promisors_tail;
> +};
[snip]
> -static struct promisor_remote *promisors;
> -static struct promisor_remote **promisors_tail = &promisors;
Nice, so all the old globals are contained in a data struct now...
> -static struct promisor_remote *promisor_remote_new(const char *remote_name)
> +static struct promisor_remote *promisor_remote_new(struct promisor_remote_config *config,
> + const char *remote_name)
...which we use during promisor_remote creation now.
> @@ -135,59 +140,63 @@ static int promisor_remote_config(const char *var, const char *value, void *data
> return 0;
> }
>
> -static int initialized;
Very happy to see this pattern vanquished. ;)
> -static void promisor_remote_init(void)
> +static void promisor_remote_init(struct repository *r)
> {
> - if (initialized)
> + struct promisor_remote_config *config;
> +
> + if (r->promisor_remote_config)
> return;
> - initialized = 1;
So it's not a "call once, set global state" anymore, but we are also
careful not to trample any existing state on the 'struct repository'.
Nice.
> + config = r->promisor_remote_config =
> + xcalloc(sizeof(*r->promisor_remote_config), 1);
Hm. Am I the only one who doesn't like assigning from the result of an
assignment like this? ...Based on 'git grep "=[^=]\+=[^=]"' yes, I am the
only one :)
Overall this patch looks OK to me, but I see that there are some changes
suggested for the next version, so I'll hold off on a reviewed-by so I
can have a look at those too.
- Emily
