[PATCH v2] config: Consistent call style to gpg settings

diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index eac901a..2e2b8d0 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c

@@ -28,7 +28,7 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
 
 static int commit_tree_config(const char *var, const char *value, void *cb)
 {
-	int status = git_gpg_config(var, value, NULL);
+	int status = git_gpg_config(var, value, cb);
 	if (status)
 		return status;
 	return git_default_config(var, value, cb);

diff --git a/builtin/log.c b/builtin/log.c
index 31f5a9e..7f02af7 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -339,6 +339,8 @@ static int cmd_log_walk(struct rev_info *rev)
 
 static int git_log_config(const char *var, const char *value, void *cb)
 {
+	int status;
+
 	if (!strcmp(var, "format.pretty"))
 		return git_config_string(&fmt_pretty, var, value);
 	if (!strcmp(var, "format.subjectprefix"))

@@ -365,8 +367,11 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		use_mailmap_config = git_config_bool(var, value);
 		return 0;
 	}
-	if (git_gpg_config(var, value, cb) < 0)
-		return -1;
+	
+	status = git_gpg_config(var, value, cb);
+	if (status)
+		return status;
+	
 	if (grep_config(var, value, cb) < 0)
 		return -1;
 	return git_diff_ui_config(var, value, cb);

diff --git a/builtin/merge.c b/builtin/merge.c
index 7c8922c..6c03eb8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c

@@ -580,7 +580,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	status = fmt_merge_msg_config(k, v, cb);
 	if (status)
 		return status;
-	status = git_gpg_config(k, v, NULL);
+	status = git_gpg_config(k, v, cb);
 	if (status)
 		return status;
 	return git_diff_ui_config(k, v, cb);

-- 
1.7.11.msysgit.0


Hans Brigman <hbrigman@openspan.com> writes:

> From: Jacob Sarvis <jsarvis@openspan.com>
>
> config: Consistent call style to gpg settings
>
> Calling style for passing settings to git_gpg_config is inconsistent 
> between commit-tree, commit, merge, tag, and verify-tag.
>
> Consolidate style of calling git_gpg_config.
>
> Signed-off-by: Hans Brigman <hbrigman@openspan.com>

Exactly the same comments as the review for the other patch apply here.

> ---
> builtin/commit-tree.c |  5 ++---
> builtin/commit.c      |  6 ++----
> builtin/merge.c       | 12 ++++--------
> builtin/tag.c         |  5 ++---
> builtin/verify-tag.c  |  5 ++---
> 5 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c index 
> eac901a..45e0152 100644
> --- a/builtin/commit-tree.c
> +++ b/builtin/commit-tree.c
> @@ -28,9 +28,8 @@ static void new_parent(struct commit *parent, struct 
> commit_list **parents_p)  static int commit_tree_config(const char 
> *var, const char *value, void *cb) {
> -              int status = git_gpg_config(var, value, NULL);
> -              if (status)
> -                              return status;
> +             if (git_gpg_config(var, value, cb) < 0)
> +                             return -1;
>                return git_default_config(var, value, cb); }

Earlier, we always returned what the underlying helper returned, but in this version, we ignore error return values from git_gpg_config() but honor error return values from git_default_config().

This is making things worse, no?