From: Junio C Hamano <gitster@pobox.com>
To: "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Glen Choo <chooglen@google.com>
Subject: Re: [PATCH 2/2] config: respect includes in protected config
Date: Wed, 12 Oct 2022 13:48:13 -0700 [thread overview]
Message-ID: <xmqqv8ookbua.fsf@gitster.g> (raw)
In-Reply-To: <0ff5b5741a519c63e65ef57d7d0b3148c38c1a52.1665603814.git.gitgitgadget@gmail.com> (Glen Choo via GitGitGadget's message of "Wed, 12 Oct 2022 19:43:34 +0000")
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
Not commenting on the code yet as I am in the middle of today's
integration run, but as I notice a bad pattern being followed, let
me comment before it spreads too widely.
The "add failing test first and then fix the code with flipping the
test to success" is very much unwelcome. For whoever gets curious
enough (me included when accepting posted patch), it is easy to
revert only the part of the commit outside t/ tentatively to see how
the original code breaks. Keeping the fix and protection of the fix
together will avoid mistakes. The post context of the hunk that
changes test_expect_failure to test_expect_success does not cover
the test script, thereby hiding the body of the test that changes
behaviour while reviewing the patch text, which is another downside.
> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
> index 720d6cdd60b..dc3496897ab 100755
> --- a/t/t0033-safe-directory.sh
> +++ b/t/t0033-safe-directory.sh
> @@ -71,7 +71,7 @@ test_expect_success 'safe.directory=*, but is reset' '
> expect_rejected_dir
> '
>
> -test_expect_failure 'safe.directory in included file' '
> +test_expect_success 'safe.directory in included file' '
> cat >gitconfig-include <<-EOF &&
> [safe]
> directory = "$(pwd)"
> diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
> index aa6a6a8c3fd..fa33839704b 100755
> --- a/t/t0035-safe-bare-repository.sh
> +++ b/t/t0035-safe-bare-repository.sh
> @@ -51,7 +51,7 @@ test_expect_success 'safe.bareRepository on the command line' '
> -c safe.bareRepository=all
> '
>
> -test_expect_failure 'safe.bareRepository in included file' '
> +test_expect_success 'safe.bareRepository in included file' '
> cat >gitconfig-include <<-EOF &&
> [safe]
> bareRepository = explicit
next prev parent reply other threads:[~2022-10-12 20:48 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-12 19:43 [PATCH 0/2] [2.38 regression] config: respect includes in protected config Glen Choo via GitGitGadget
2022-10-12 19:43 ` [PATCH 1/2] t0033, t0035: test for included config Glen Choo via GitGitGadget
2022-10-12 22:28 ` Ævar Arnfjörð Bjarmason
2022-10-12 19:43 ` [PATCH 2/2] config: respect includes in protected config Glen Choo via GitGitGadget
2022-10-12 20:48 ` Junio C Hamano [this message]
2022-10-12 22:09 ` Glen Choo
2022-10-12 21:07 ` Junio C Hamano
2022-10-12 19:47 ` [PATCH 0/2] [2.38 regression] " Glen Choo
2022-10-12 20:54 ` Junio C Hamano
2022-10-13 17:43 ` [PATCH v2] " Glen Choo via GitGitGadget
2022-10-13 18:21 ` Glen Choo
2022-10-14 20:14 ` Jeff King
2022-10-18 14:36 ` Johannes Schindelin
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqv8ookbua.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
/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.
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 NNTP newsgroup(s).