[v5,3/3] tag: use new advice API to check visibility
diff mbox series

Message ID 01b195ebe1d2ab1593915318b7e8adbee5d9614c.1582628141.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • advice: revamp advise API
Related show

Commit Message

Derrick Stolee via GitGitGadget Feb. 25, 2020, 10:55 a.m. UTC
From: Heba Waly <heba.waly@gmail.com>

Following the new helpers added to the advice library,
replace the global variable check approach by the new
API calls

Signed-off-by: Heba Waly <heba.waly@gmail.com>
---
 advice.c       | 2 --
 advice.h       | 1 -
 builtin/tag.c  | 5 +++--
 t/t7004-tag.sh | 1 +
 4 files changed, 4 insertions(+), 5 deletions(-)

Comments

Junio C Hamano Feb. 25, 2020, 5:48 p.m. UTC | #1
"Heba Waly via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Heba Waly <heba.waly@gmail.com>
>
> Following the new helpers added to the advice library,
> replace the global variable check approach by the new
> API calls

The last paragraph of the proposed log message you had for [2/3]
described that this step is just an example better than the above
one, which would leave readers puzzled what our plans are for dozens
of existing advise() calls.

> +	if (type == OBJ_TAG)
> +		advise_if_enabled(NESTED_TAG, _(message_advice_nested_tag),
> +				  tag, object_ref);

This is probably a good enough example why OBJ_TAG is a good name
but NESTED_TAG is not---type could be something different from
OBJ_TAG but the other possiblities are all OBJ_<object-type>.  We
want the advice types to have the same property.

Patch
diff mbox series

diff --git a/advice.c b/advice.c
index 5c2068b8f8a..ea6e65c1ce0 100644
--- a/advice.c
+++ b/advice.c
@@ -29,7 +29,6 @@  int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 int advice_graft_file_deprecated = 1;
 int advice_checkout_ambiguous_remote_branch_name = 1;
-int advice_nested_tag = 1;
 int advice_submodule_alternate_error_strategy_die = 1;
 
 static int advice_use_color = -1;
@@ -89,7 +88,6 @@  static struct {
 	{ "waitingForEditor", &advice_waiting_for_editor },
 	{ "graftFileDeprecated", &advice_graft_file_deprecated },
 	{ "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name },
-	{ "nestedTag", &advice_nested_tag },
 	{ "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die },
 
 	/* make this an alias for backward compatibility */
diff --git a/advice.h b/advice.h
index a8461a362a3..509b562edb1 100644
--- a/advice.h
+++ b/advice.h
@@ -29,7 +29,6 @@  extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
 extern int advice_graft_file_deprecated;
 extern int advice_checkout_ambiguous_remote_branch_name;
-extern int advice_nested_tag;
 extern int advice_submodule_alternate_error_strategy_die;
 
 /*
diff --git a/builtin/tag.c b/builtin/tag.c
index e0a4c253828..45e959d5f8f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -231,8 +231,9 @@  static void create_tag(const struct object_id *object, const char *object_ref,
 	if (type <= OBJ_NONE)
 		die(_("bad object type."));
 
-	if (type == OBJ_TAG && advice_nested_tag)
-		advise(_(message_advice_nested_tag), tag, object_ref);
+	if (type == OBJ_TAG)
+		advise_if_enabled(NESTED_TAG, _(message_advice_nested_tag),
+				  tag, object_ref);
 
 	strbuf_addf(&header,
 		    "object %s\n"
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 6db92bd3ba6..74b637deb25 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1726,6 +1726,7 @@  test_expect_success 'recursive tagging should give advice' '
 	hint: already a tag. If you meant to tag the object that it points to, use:
 	hint: |
 	hint: 	git tag -f nested annotated-v4.0^{}
+	hint: Disable this message with "git config advice.nestedTag false"
 	EOF
 	git tag -m nested nested annotated-v4.0 2>actual &&
 	test_i18ncmp expect actual