On Thu, May 24, 2018 at 4:12 PM, Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> On Thu, May 24 2018, Eric Sunshine wrote:
>> On Thu, May 24, 2018 at 3:35 PM, Ævar Arnfjörð Bjarmason
>> <avarab@xxxxxxxxx> wrote:
>>> fsck.skipList::
>>> + 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?
I see where you're coming from, however, I would think that readers
arriving at this topic (generally) do so as a result of actively
looking for it (as opposed to happening upon it), in which case they
probably are directly seeking information about it; the incidental
information is just a bonus after reading what they came to learn.
Anyhow, I don't care too strongly about it (it was just a "nit", after all).
