report build failures as failures #675
Conversation
|
I don't think timed out should be a failure. That's something that happens extremely often (basically any PR targeting staging will time out on Darwin), and getting people used to merging despite a reported failure will cause alarm fatigue and make it more likely that real failures are ignored. |
There was a problem hiding this comment.
Can we distinguish failures of the leaf OfBorg was trying to build from failures of dependencies? Another thing that comes up a lot especially on staging is failures of dependencies due to flaky tests or whatever, and those shouldn't block a PR being merged, so they shouldn't show up as failures.
I can't answer the first question as I don't know enough about the entire system. I don't think PRs are blocked regardless on failing ofborg checks. |
Not on a technical level, but people get very nervous about ignoring a failure when merging, and in practice will very rarely do it without being very sure it's okay. That's a useful property that we should preserve, and if we lose it by accident, we won't get a second chance. |
|
See related discussion here: https://discourse.nixos.org/t/can-we-make-ofborg-show-broken-tests-as-red/10717/2 |
|
Happy to reopen at any time. |
|
Any reason you closed it, @adamcstephens ? I thought this was a worthwile idea and I hope someone will pick it up someday! |
It is far too easy to miss clear failures when we report them as neutral. Not only does this not register them in GitHub visually as a failure, but once all checks have completed successfully GitHub collapses the checks UI and shows the PR as fully successfully checked.
We're already merging PRs that are failing checks, it's just not as visible. This change will at least provide visibility into those failing checks, to allow us to make better informed decisions when proceeding to merge anyway.
Note: I have little knowledge of what I'm doing, but this seems to be the correct place to change this.