Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix status check not working for PRs without any status checks #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Skipants
Copy link

@Skipants Skipants commented May 3, 2024

The Github GraphQL response returns null for the StatusCheckRollup object if there are no status checks at all. In that case, this should be a pass.

From my understanding, spr gets the state of the PullRequest with a graphql query like the following:

gh api graphql -f query='
  query PullRequest($repo_owner: String!, $repo_name: String!, $pr_number: Int!) {
    viewer {
      login
    }
    repository(owner: $repo_owner, name: $repo_name) {
      pullRequest(number: $pr_number) {
        id
        number
        title
        body
        baseRefName
        headRefName
        mergeable
        reviewDecision
        repository {
          id
        }
        commits(first: 100) {
          nodes {
            commit {
              oid
              messageHeadline
              messageBody
              statusCheckRollup {
                state
              }
            }
          }
        }
      }
    }
  }' -f repo_owner='myrepo' -f repo_name='myrepo' -f pr_number=123

On the repository I am working on, this would give a response with a statusCheckRollup: null:

{
  "data": {
    "viewer": {
      "login": "Skipants"
    },
    "repository": {
      "pullRequest": {
        "id": "PR_fakeid123",
        "number": 123,
        "title": "My Cool PR",
        "body": "My Cool PR Stack\n\n---\n\n**Stack**:\n- #456\n- #123 ⬅\n\n\n⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do not merge manually using the UI - doing s
o may have unexpected results.*",
        "baseRefName": "master",
        "headRefName": "spr/master/abc123",
        "mergeable": "MERGEABLE",
        "reviewDecision": "APPROVED",
        "repository": {
          "id": "thisIsAFakeID="
        },
        "commits": {
          "nodes": [
            {
              "commit": {
                "oid": "123abc",
                "messageHeadline": "This is a cool commit",
                "messageBody": "Commit message\n\ncommit-id
:abc123",
                "statusCheckRollup": null
              }
            }
          ]
        }
      }
    }
  }
}

This prevented me from merging PRs in the stack as the status check would always fail. This fix makes null statusCheckRollups a pass instead.

I rebuilt this executable locally and was able to move my PRs along with no issue.

The Github GraphQL response returns null for the StatusCheckRollup object if there are no status checks at all. In that case, this should be a pass.
@Skipants Skipants marked this pull request as draft May 3, 2024 20:11
@Skipants
Copy link
Author

Skipants commented May 3, 2024

I forgot to run tests and I have a feeling they might fail on this change. I changed this to a draft PR for the time-being.

@Skipants Skipants marked this pull request as ready for review May 6, 2024 13:46
@Skipants
Copy link
Author

Skipants commented May 6, 2024

Tests pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant