Re: [PATCH v2 1/2] sequencer: fix gpg option passed to merge subcommand

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> What is "natural" is subjective but what is the default config is
> not. If test scripts set random config variables and assumes that they
> will be ...
> ... cleared how are new contributors supposed to figure it out? If each
> test starts with the default config it is much easier to reason about
> it.

If this were a test script with many tests, all of which depend on
starting from clean slate wrt the configuration variable space, and
if you are adding just one or two tests that wants to run with
configuration variable in effect, the story would be quite
different.  For example, I would think it would make a lot of sense
in <20201014232517.3068298-1-emilyshaffer@google.com> to use
test_config instead of "git config" as it is clear that leading 100+
lines of tests run with the default configuration, and it is sane to
expact that later tests that may be added in the future would be the
same.

Look at the test script in question again and notice that it is
about seeing behaviour with the single configuration variable set to
true or false.  Using test_config would still signal the test pieces
that use it do depend on the shown setting of the variable, but it
does not help at all the test pieces that wants to see what happens
when there is no configuration.  Explicitly using "git config" and
"test_unconfig" actually would _help_ reviewers who only looks
between the pre- and post- context lines that are affected by the
patch, as they do not have to know or assume that "normal state is
nothing configured" (which needs to be verified by seeing all the
existing tests that are not shown in the patch use test_config to
clear the setting when they are done).

All of the above would be obvious once you think about it, I'd
think.

Thanks.