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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 27, 2018, 21:28 UTC
Message-ID: <xmqqo9esl66u.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180727143720.14948-9-avarab@gmail.com>

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

Show 7 quoted lines

> 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.

Show 18 quoted lines

>
> 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.