Johannes Schindelin <johannes.schindelin@xxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in