Hi,
It's been a while since I looked at this series. Hopefully I can
come at it with some fresh eyes. Thanks for your perseverance.
Stefan Beller wrote:
> We need the submodule update strategies in a later patch.
This description doesn't explain what the patch will do or why
parse_config didn't already retain the value. If I look back
at this patch later and want to understand why it was written,
what would I want to know?
It could say
Currently submodule.<name>.update is only handled by git-submodule.sh.
C code will start to need to make use of that value as more of the
functionality of git-submodule.sh moves into library code in C.
Add the update field to 'struct submodule' and populate it so it can
be read as sm->update.
[...]
> +++ b/submodule-config.c
> @@ -311,6 +312,16 @@ static int parse_config(const char *var, const char *value, void *data)
> free((void *) submodule->url);
> submodule->url = xstrdup(value);
> }
> + } else if (!strcmp(item.buf, "update")) {
> + if (!value)
> + ret = config_error_nonbool(var);
> + else if (!me->overwrite && submodule->update != NULL)
> + warn_multiple_config(me->commit_sha1, submodule->name,
> + "update");
> + else {
> + free((void *) submodule->update);
> + submodule->update = xstrdup(value);
> + }
(not about this patch) This code is repetitive. Would there be a way
to share code between the parsing of different per-submodule settings?
[...]
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -14,6 +14,7 @@ struct submodule {
> const char *url;
> int fetch_recurse;
> const char *ignore;
> + const char *update;
gitmodules(5) tells me the only allowed values are checkout, rebase,
merge, and none. I wouldn't know at a glance how to match against
those in calling code. Can this be an enum?
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html