Merged
Conversation
Previously, we only looked at check_runs status when the combined status was empty. Now we look at the native GitHub actions check_runs in any case, and even avoid an API call if check runs have failed.
The API actually returns the expected amount of checks, so rely on that vs. on len(runs). Also, only return the short status so matching becomes more elegant (plus we don't use that information anywhere).
Collaborator
Author
|
@schuellerf @marusak @achilleas-k FYI, this improves CI status matching, but there's still a corner case left. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These two commits ensure that our CI status matching improves, however, it's still not perfect.
Previously, we only looked at check_runs status when the
combined_statuswas empty. Now we look at the native GitHub actions check_runs in any case, and even avoid an API call if check runs have failed.This means that effectively we create our own
combined_statusout of external CIs and check runs.There is still a corner case I'm unsure how to handle. When a PR has checks defined, but they haven't been triggered (I presume that external contributors don't have the permissions in this case?), the API returns
total_count = 0.So while the correct state would be
pending, we cannot know if there are maybe no GitHub checks defined or if they haven't been triggered.Here's a PR that I ran into with this problem: osbuild/cloud-image-val#283
It's currently part of the queue, even though CI hasn't run.
We can of course also decide to ignore this corner case because there may not be a pretty solution to it.