Disable CI status change comments unless the "automerge" label is used #578
Conversation
Codecov Report
@@ Coverage Diff @@
## main #578 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 1032 1017 -15
Branches 72 71 -1
=========================================
- Hits 1032 1017 -15
|
| if failure: | ||
| if failure or not success: | ||
| emoji = "❌" | ||
| status = "it's a failure or timed out" | ||
| elif not success: | ||
| emoji = "❌" | ||
| status = "it's a failure" |
There was a problem hiding this comment.
Was this change intentional?
@DanielNoord recently fixed this.
There was a problem hiding this comment.
@ambv Was this indeed intentional? If not I could open a PR to revert it to my last fix?
There was a problem hiding this comment.
@ezio-melotti I checked in cpython and it seems to be okay, although there are some weird comments that are probably due to checks timing out and then being rerun.
It doesn't regress on #576 but it does remove the narrowing down that the "it's a failure" comment gives. Previously if that was the only thing in the comment you knew there wasn't a timeout, so there was likely an actual issue with the PR. You no longer get this information, but I'm not sure it warrants a PR to re-introduce that. It doesn't seem to helpful anyway.
That said, it could be simplified to if not success. If failure == True then success will always be False, so that doesn't need double-checking. Want me to open a PR for that?
There was a problem hiding this comment.
In python/cpython#97820 (comment) I got the Status check is done, and it's a failure or timed out ❌. message, even though there was no evident failure. python/cpython#97821 (comment) reported success.
There was a problem hiding this comment.
It's hard to debug these comments after the fact.
https://api.github.com/repos/python/cpython/commits/66eca5d2f185cd6943afeb69dae3eba625bbd077/status
https://api.github.com/repos/python/cpython/commits/66eca5d2f185cd6943afeb69dae3eba625bbd077/check-runs
Both show that there are no failed runs. So I don't know what is going on there honestly... But I'm also fairly sure it isn't caused by the changes in this PR.
There was a problem hiding this comment.
I cannot find it anymore but I had an example where it was a timeout but TIMED_OUT was not set so the bot would report a failure. And since the best the message could say anyway was that it's either a failure or a timeout, I decided to just shorten it.
IIRC this change also let me not touch more tests, which otherwise I would have to alter due to the leave_comment=False change.
What's more worrying is that @ezio-melotti's example are still the bot leaving comments even those leave_comment=False is set. What gives?
There was a problem hiding this comment.
Turns out this was due to a missing change that I now also cover in #583
There was a problem hiding this comment.
Also, I remember what I meant by "this change also let me not touch more tests". The change in this PR caused test coverage to fall due to one of those branches to not be exercised anymore.
Works around #577