git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Ariadne Conill <ariadne@dereferenced.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 3/3] tests: defang pager tests by explicitly disabling the log.mailmap warning
Date: Sun, 14 Jul 2019 14:56:46 -0700	[thread overview]
Message-ID: <xmqqy31046w1.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190712230204.16749-4-ariadne@dereferenced.org> (Ariadne Conill's message of "Fri, 12 Jul 2019 18:02:04 -0500")

Ariadne Conill <ariadne@dereferenced.org> writes:

> In the previous patch, we added a deprecation warning for the current
> log.mailmap setting. This warning only appears when git is attached to
> a controlling terminal. Some tests however run under an emulated
> terminal, so we need to disable the warning for those tests.
>
> Signed-off-by: Ariadne Conill <ariadne@dereferenced.org>
> ---
>  t/t7006-pager.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Hmm, this is horrible.  

These tests are primarily to see how use of color gets affected by
various configuration and the use of the pager, and having to
sprinkle log.mailmap configuration to just randomly selected 10
tests among 50+ tests in the script makes readers wonder if the
configuration has anything to do with the coloring (answer: no).

The primary reason why other 40+ tests do not need log.mailmap
tweaked is not because log.mailmap does not affect the coloring.
But for a new developer who will be adding a new test to this file,
how would s/he decide if the new test needs log.mailmap=false like
these 10, or it is like the other 40+?

It almost makes me feel that it would be much better to just disable
the warning inside the setup part, perhaps like

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 00e09a375c..283de499fc 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -7,6 +7,8 @@ test_description='Test automatic use of a pager.'
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
 test_expect_success 'setup' '
+	: squelch advise messages during the transition &&
+	git config --global log.mailmap false &&
 	sane_unset GIT_PAGER GIT_PAGER_IN_USE &&
 	test_unconfig core.pager &&
 

      reply	other threads:[~2019-07-14 21:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12 23:02 [PATCH v2 0/3] document deprecation of log.mailmap=false default Ariadne Conill
2019-07-12 23:02 ` [PATCH v2 1/3] log: add warning for unspecified log.mailmap setting Ariadne Conill
2019-07-14 21:55   ` Junio C Hamano
2019-07-12 23:02 ` [PATCH v2 2/3] documentation: mention --no-use-mailmap and log.mailmap false setting Ariadne Conill
2019-07-12 23:02 ` [PATCH v2 3/3] tests: defang pager tests by explicitly disabling the log.mailmap warning Ariadne Conill
2019-07-14 21:56   ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy31046w1.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=ariadne@dereferenced.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).