From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 02 Dec 2025 07:53:28 GMT
Subject: Re: [PATCH v2 1/5] scalar: annotate config file with "set by scalar"
Message-ID: <aS6a-K--2acXnIFY@pks.im>
In-Reply-To: <639ff98c44c45bb570fdde5123127aa099c299f8.1764607847.git.gitgitgadget@gmail.com>

On Mon, Dec 01, 2025 at 04:50:43PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh
> index bd6f0c40d2..43c210a23d 100755
> --- a/t/t9210-scalar.sh
> +++ b/t/t9210-scalar.sh
> @@ -210,6 +210,9 @@ test_expect_success 'scalar reconfigure' '
>  	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
>  	test_path_is_file one/src/cron.txt &&
>  	test true = "$(git -C one/src config core.preloadIndex)" &&
> +	test_grep "preloadIndex = true # set by scalar" one/src/.git/config &&
> +	test_grep "excludeDecoration = refs/prefetch/\* # set by scalar" one/src/.git/config &&
> +
>  	test_subcommand git maintenance start <reconfigure &&
>  	test_subcommand ! git maintenance unregister --force <reconfigure &&

We _could_ make this a bit more solid by adding a test that:

  1. Initializes a new repository.

  2. Saves the configuration.

  3. Performs `scalar reconfigure`.

  4. Asserts that all new non-section-header lines in the configuration
     have a trailing "#set by scalar" comment.

This would ensure that there is no callsite we forgot to add the new
annotation to, and that there are new future callsites where somebody
isn't aware of the comments.

I don't insist on such a test though, so please feel free to ignore this
suggestion.

Patrick