Re: [PATCH 2/4] config doc: unify the description of fsck.* and receive.fsck.*

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 24, 2018, 20:12 UTC
Message-ID: <87r2m07qz6.fsf@evledraar.gmail.com>
In-Reply-To: <CAPig+cTr2J6yj39NNdV1vT8CQP2qyPxNB1ggaE9bGgXukuq1yQ@mail.gmail.com>

On Thu, May 24 2018, Eric Sunshine wrote:

Show 37 quoted lines

> On Thu, May 24, 2018 at 3:35 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> The documentation for the fsck.<msg-id> and receive.fsck.<msg-id>
>> variables was mostly duplicated in two places, with fsck.<msg-id>
>> making no mention of the corresponding receive.fsck.<msg-id>, and the
>> same for fsck.skipList.
>> [...]
>> Rectify this situation by describing the feature in general terms
>> under the fsck.* documentation, and make the receive.fsck.*
>> documentation refer to those variables instead.
>> [...]
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> @@ -1554,23 +1554,41 @@ filter.<driver>.smudge::
>>  fsck.skipList::
>> -       The path to a sorted list of object names (i.e. one SHA-1 per
>> -       line) that are known to be broken in a non-fatal way and should
>> -       be ignored. This feature is useful when an established project
>> -       should be accepted despite early commits containing errors that
>> -       can be safely ignored such as invalid committer email addresses.
>> -       Note: corrupt objects cannot be skipped with this setting.
>> +       Like `fsck.<msg-id>` this variable has a corresponding
>> +       `receive.fsck.skipList` variant.
>> ++
>> +The path to a sorted list of object names (i.e. one SHA-1 per line)
>> +that are known to be broken in a non-fatal way and should be
>> +ignored. This feature is useful when an established project should be
>> +accepted despite early commits containing errors that can be safely
>> +ignored such as invalid committer email addresses. Note: corrupt
>> +objects cannot be skipped with this setting.
>
> Nit: This organization seems backward. Typically, one would describe
> what the option is for and then add the incidental note ("Like
> fsck.<...>, this variable...") at the end. It's not clear why this
> patch demotes the description to a secondary paragraph and considers
> the incidental note as primary.

I could change it like that. I was thinking that later in the series fetch.fsck.* is going to be first in the file, and then the user is told to look at this variable, so it made sense to note from the outset that we're describing several variables here.

What do you think?