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!
quoted hunk ↗ jump to hunk
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...
quoted hunk ↗ jump to hunk
-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.
quoted hunk ↗ jump to hunk
@@ -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. ;)
quoted hunk ↗ jump to hunk
-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