[PATCH] D96371: [lit] Add --ignore-fail

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added a comment.

In D96371#2578424 <https://reviews.llvm.org/D96371#2578424>, @jhenderson wrote:

> In D96371#2568808 <https://reviews.llvm.org/D96371#2568808>, @jdenny wrote:
>
>>> I think the lit documentation needs updating to mention this change?
>>
>> You mean lit.rst?  I'm happy to put it there.  However, first, what is the threshold in general?  Many options are missing there.  Is that intentional or by accident?
>
> At least in the LLVM binutils, the threshold has been "any option", and the only way to catch it is to make sure reviewers remember to mention it in the review. I think it's likely the missing options are all accidental, but I haven't dug into the history to confirm this one way or the other.
>
>> (If it's always by accident, we really ought to consider generating that part from `lit --help` so we don't have to maintain two versions of the same thing.  That should be a different patch of course.)
>
> I am not opposed to someone doing that, but good luck getting it to work nicely (complete with sub-sectioning as is already present). Any solution should be applied more generally across the other tools with documentation in the Command Guide, I think.

Agreed, but we could start with one tool and extend to others as we figure out the best approach.

> I also wouldn't say that the help text and the documentation should always match, so they're not always two places for the same thing. In general, the documentation can be more detailed, with e.g. examples, which don't really belong in the help text.

Perhaps there could be some way to override/extend the help text for specific options.

Well, for now, I've added `--ignore-fail` to lit.rst, and I'm not going to work on this more general approach.  Maybe later.  Thanks for presenting your thoughts.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96371/new/

https://reviews.llvm.org/D96371