"Heba Waly via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> +static struct {
> + const char *key;
> + int enabled;
> +} advice_setting[] = {
> + [ADVICE_ADD_EMBEDDED_REPO] = { "addEmbeddedRepo", 1 },
It would be nicer to future developers to flip the polarity, as we
do not have to write 1 all over the place, especially if we plan to
extend the structure over time and to use designated initializers
for only certain fields:
static struct {
const char *key;
int disabled;
} advice_setting[] = {
[ADDVICE_ADD_EMBEDDED_REPO] = { .key = "addEmbeddedRepo" },
> @@ -149,6 +218,13 @@ int git_default_advice_config(const char *var, const char *value)
> if (strcasecmp(k, advice_config[i].name))
> continue;
> *advice_config[i].preference = git_config_bool(var, value);
> + break;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(advice_setting); i++) {
> + if (strcasecmp(k, advice_setting[i].key))
> + continue;
> + advice_setting[i].enabled = git_config_bool(var, value);
> return 0;
Turning this into "break;" would make it similar to the loop before
this one, and will allow other people to add more code after this
loop later.
> +int cmd__advise_if_enabled(int argc, const char **argv)
> +{
> + if (!argv[1])
> + die("usage: %s <advice>", argv[0]);
> +
> + setup_git_directory();
> + git_config(git_default_config, NULL);
> +
> + /*
> + Any advice type can be used for testing, but NESTED_TAG was selected
> + here and in t0018 where this command is being executed.
> + */
Style (will fix up locally).
Thanks. I think this is reasonable with or without the suggested
fixes.
