{"thread":{"id":"33305","subject":"[PATCH v2] config: Consistent call style to gpg settings","startedAt":"2013-03-27T17:19:08Z","lastAt":"2013-03-27T17:19:08Z","messageCount":1,"participants":["Hans Brigman"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"212412","messageId":"8C726954D36902459248B0627BF2E66F45D70C3FCE@AUSP01VMBX10.collaborationhost.net","threadId":"33305","inReplyTo":null,"subject":"[PATCH v2] config: Consistent call style to gpg settings","fromName":"Hans Brigman","fromEmail":"hbrigman@openspan.com","sentAt":"2013-03-27T17:19:08Z","receivedAt":"2013-03-27T17:19:08Z","isPatch":true,"sender":{"key":"hbrigman@openspan.com","avatar":null},"body":"From: Jacob Sarvis <jsarvis@openspan.com>\n\nCalling style for passing settings to git_gpg_config is inconsistent\nbetween commit-tree, commit, log, merge, tag, and verify-tag.\n\nConsolidate style of calling git_gpg_config.\n\nSigned-off-by: Jacob Sarvis <jsarvis@openspan.com>\nSigned-off-by: Hans Brigman <hbrigman@openspan.com>\n---\n builtin/commit-tree.c | 2 +-\n builtin/log.c         | 9 +++++++--\n builtin/merge.c       | 2 +-\n 3 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit-tree.c b/builtin/commit-tree.c\nindex eac901a..2e2b8d0 100644\n--- a/builtin/commit-tree.c\n+++ b/builtin/commit-tree.c\n@@ -28,7 +28,7 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)\n \n static int commit_tree_config(const char *var, const char *value, void *cb)\n {\n-\tint status = git_gpg_config(var, value, NULL);\n+\tint status = git_gpg_config(var, value, cb);\n \tif (status)\n \t\treturn status;\n \treturn git_default_config(var, value, cb);\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 31f5a9e..7f02af7 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -339,6 +339,8 @@ static int cmd_log_walk(struct rev_info *rev)\n \n static int git_log_config(const char *var, const char *value, void *cb)\n {\n+\tint status;\n+\n \tif (!strcmp(var, \"format.pretty\"))\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n@@ -365,8 +367,11 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tuse_mailmap_config = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\tif (git_gpg_config(var, value, cb) < 0)\n-\t\treturn -1;\n+\t\n+\tstatus = git_gpg_config(var, value, cb);\n+\tif (status)\n+\t\treturn status;\n+\t\n \tif (grep_config(var, value, cb) < 0)\n \t\treturn -1;\n \treturn git_diff_ui_config(var, value, cb);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 7c8922c..6c03eb8 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -580,7 +580,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n \tstatus = fmt_merge_msg_config(k, v, cb);\n \tif (status)\n \t\treturn status;\n-\tstatus = git_gpg_config(k, v, NULL);\n+\tstatus = git_gpg_config(k, v, cb);\n \tif (status)\n \t\treturn status;\n \treturn git_diff_ui_config(k, v, cb);\n-- \n1.7.11.msysgit.0\n\n\nHans Brigman <hbrigman@openspan.com> writes:\n\n> From: Jacob Sarvis <jsarvis@openspan.com>\n>\n> config: Consistent call style to gpg settings\n>\n> Calling style for passing settings to git_gpg_config is inconsistent \n> between commit-tree, commit, merge, tag, and verify-tag.\n>\n> Consolidate style of calling git_gpg_config.\n>\n> Signed-off-by: Hans Brigman <hbrigman@openspan.com>\n\nExactly the same comments as the review for the other patch apply here.\n\n> ---\n> builtin/commit-tree.c |  5 ++---\n> builtin/commit.c      |  6 ++----\n> builtin/merge.c       | 12 ++++--------\n> builtin/tag.c         |  5 ++---\n> builtin/verify-tag.c  |  5 ++---\n> 5 files changed, 12 insertions(+), 21 deletions(-)\n>\n> diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c index \n> eac901a..45e0152 100644\n> --- a/builtin/commit-tree.c\n> +++ b/builtin/commit-tree.c\n> @@ -28,9 +28,8 @@ static void new_parent(struct commit *parent, struct \n> commit_list **parents_p)  static int commit_tree_config(const char \n> *var, const char *value, void *cb) {\n> -              int status = git_gpg_config(var, value, NULL);\n> -              if (status)\n> -                              return status;\n> +             if (git_gpg_config(var, value, cb) < 0)\n> +                             return -1;\n>                return git_default_config(var, value, cb); }\n\nEarlier, 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().\n\nThis is making things worse, no?\n"}]}