Re: [PATCH v3 08/10] fsck: test & document {fetch,receive}.fsck.* config fallback

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Test and document that the {fetch,receive}.fsck.* family of variables
> doesn't fall back on the corresponding .fsck.* variables.
>
> This was alluded to in the existing documentation by saying that
> "receive" looks at receive.fsck.* and "fsck" looks at fsck.* etc., but
> it wasn't explicitly stated that there was no fallback, and if you'd
> e.g. like to configure the skipList you need to do that for all three.

Hmph, personally I felt that the updates so far made it clear and
explicit enough.

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/config.txt        | 12 ++++++++++++
>  t/t5504-fetch-receive-strict.sh | 26 ++++++++++++++++++++++++--
>  2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 8dace49daa..57c463c6e2 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1619,6 +1619,12 @@ The rest of the documentation discusses `fsck.*` for brevity, but the
>  same applies for the corresponding `receive.fsck.*` and
>  `fetch.<msg-id>.*`. variables.
>  +
> +Unlike variables like `color.ui` and `core.editor` the
> +`receive.fsck.<msg-id>` and `fetch.fsck.<msg-id>` variables will not
> +fall back on the `fsck.<msg-id>` configuration if they aren't set. To

I do not think "Unlike ..." and mention of two sample variables
(color.ui and core.editor) adds much value to the paragraph,
primarily because it does not mention "color.status" etc. to show
what kind of fallback the sentence wants to discuss.  I actually
find the paragraph even clearer to read if it began with "The
`receive.fsck.<msgid>` and ...".

If you insist keeping that "Unlike..." thing, please have a comma
before "the `receive.fsck.<msg-id>`", but I'd recommend to remove
it.

> +uniformly configure the same fsck settings in different circumstances
> +all three of them they must all set to the same values.

That is not incorrect per-se, but I doubt it is a good idea to give
an impression that most people would want al three to be the same.

You want fsck.<msg-id>.* to be stricter to ensure you do not create
new problems, together with fsck.skipList to pardon mistakes that
have already happened, and with $transfer.fsck.<msg-id>, optionally
loosen certain class of breakage if a particular project has special
needs (e.g. the tool they use is known to produce incorrect
objects).  Which makes me expect that in most cases, fsck.<msg-id>.*
would be empty, fsck.skipList to be populated with known-bad ones,
and $transfer.fsck.<msg-id> may be empty, or may not be empty.

On the other hand, there may be value in allowing skipList to be
shared across three, though.  You may have known-bad ones and you'd
need to give a copy of that known-bad thing to other participants of
the project anyway.