Subject: Re: [PATCH v6 08/19] fsck: Make fsck_commit() warn-friendly

From: Junio C Hamano <[email-masked]>
Date: Fri, 19 Jun 2015 14:01:16 -0700

Johannes Schindelin <[email-masked]> writes:

>> I do not think this "if (err) return err;" that uses the return
>> value of report(), makes sense.
>> 
>> As all the errors that use this pattern are isolated ones that does
>> not break parsing of the remainder (e.g. author ident had an extra >
>> in it may break "author " but that does not prevent us from checking
>> "committer ").
>> 
>> Your report() switches its return value based on the user setting;
>> specifically, it returns 0 if the user tells us to ignore/skip or
>> warn.  Which means that the user will see all warnings, but we stop
>> at the first error.
>> 
>> Shouldn't we continue regardless of the end-user setting in order to
>> show errors on other fields, too?
>
> I can make that happen, but please note that this is a change of
> behavior: we always stopped upon the first error.

Yeah, and we always died when we saw error, without giving users an
option to turn it down.  So?

> It was my intention not to change behavior in that way without a
> proper reason, and I saw none.

What would be the end-user experience if you stopped at the first
error?  You see an error, add an "fsck.<msg-id> = ignore" and rerun,
only to find another error and rinse and repeat?  Wouldn't you
rather see all of them and add the "ignore" to cover them in one go?

> I actually see a really good reason to *keep* the current behavior:
> one of the most prominent users of this code path is `git receive-pack
> --strict`. It is used heavily by GitHub to ensure at least a certain
> level of validity of pushed objects. Now, for this use case it is easy
> to see that you want to stop *as soon as an error was
> encountered*. And as GitHub sponsors my work on this patch series, my
> main aim is to support their use case.

While I understand that use case, I do not think stopping after
showing three more errors in a single commit would make much
difference in the bigger picture.