Re: [PATCH 2/5] implement submodule config cache for lookup of submodule names

[Heiko Voigt <hvoigt@hvoigt.net>]

On Thu, Jun 12, 2014 at 02:58:20PM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
> > ...
> > +static int is_cache_init = 0;
> 
> Please don't initialise variables in the .bss to zero by hand.

Ok will remove that.

> > + ...
> > +	warning("%s:.gitmodules, multiple configurations found for "
> > +			"submodule.%s.%s. Skipping second one!",
> > +			commit_string, name, option);
> > +}
> > + ...
> > +		if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
> > +		    strcmp(value, "all") && strcmp(value, "none")) {
> > +			warning("Invalid parameter \"%s\" for config option "
> > +					"\"submodule.%s.ignore\"", value, var);
> > +			goto release_return;
> > +		}
> 
> These two look inconsistent in different ways.  I think we typically
> quote the names like so:
> 
> 	warning("I have trouble with variable '%s' somehow", var);

Ok will change the quotation for the variable and parameter names.

Here is the fixup[1] I will queue in my branch.

Cheers Heiko

[1] ---8<----
Subject: [PATCH] fixup! implement submodule config cache for lookup of
 submodule names

---
 submodule-config.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 437fbdb..f330ccc 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -26,7 +26,7 @@ struct submodule_entry {
 };
 
 static struct submodule_cache cache;
-static int is_cache_init = 0;
+static int is_cache_init;
 
 static int config_path_cmp(const struct submodule_entry *a,
 			   const struct submodule_entry *b,
@@ -230,7 +230,7 @@ static void warn_multiple_config(const unsigned char *commit_sha1,
 	if (commit_sha1)
 		commit_string = sha1_to_hex(commit_sha1);
 	warning("%s:.gitmodules, multiple configurations found for "
-			"submodule.%s.%s. Skipping second one!",
+			"'submodule.%s.%s'. Skipping second one!",
 			commit_string, name, option);
 }
 
@@ -298,8 +298,8 @@ static int parse_config(const char *var, const char *value, void *data)
 		}
 		if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
 		    strcmp(value, "all") && strcmp(value, "none")) {
-			warning("Invalid parameter \"%s\" for config option "
-					"\"submodule.%s.ignore\"", value, var);
+			warning("Invalid parameter '%s' for config option "
+					"'submodule.%s.ignore'", value, var);
 			goto release_return;
 		}
 
-- 
2.0.0