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

Bug: does not observe required checks #73

Closed
hfaulds opened this issue Nov 14, 2022 · 3 comments · Fixed by #241
Closed

Bug: does not observe required checks #73

hfaulds opened this issue Nov 14, 2022 · 3 comments · Fixed by #241
Labels
enhancement New feature or request

Comments

@hfaulds
Copy link

hfaulds commented Nov 14, 2022

At the moment deploys are blocked on all status checks passing, even if they're not required.

This is because statusCheckRollup.state doesn't care about whether a check is required or not:

https://github.com/github/branch-deploy/blob/43afd75c657a06c55c953cc7af493d9cf565aa43/src/functions/prechecks.js#LL151

I don't know if there's an elegant way to check this 😢 (especially one that avoids paging through results). The best graphql I could come up with was this:

{
  repository(owner: "github", name: "branch-deploy") {
    pullRequest(number: 69) {
      reviewDecision
      commits(last: 1) {
        nodes {
          commit {
            checkSuites {
              totalCount
            }
            statusCheckRollup {
              state
              contexts(first: 100) {
                nodes {
                  ... on CheckRun {
                    isRequired(pullRequestNumber: 69)
                    conclusion
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}
@GrantBirki
Copy link
Member

@hfaulds Some folks might actually want all checks rather than just the required ones. How would you feel about a new input parameter like:

checks: all

requires all checks to pass no matter what

and:

checks: required

only wait for required checks


Depending on which input param is used, that determines which graphql query we make to determine if we are 🟢 or not for the deploy.

Thoughts?

@hfaulds hfaulds changed the title Bug: does not observe require checks Bug: does not observe required checks Nov 14, 2022
@hfaulds
Copy link
Author

hfaulds commented Nov 14, 2022

@GrantBirki introducing checks: all and checks:required makes a ton of sense to me 👍

@GrantBirki
Copy link
Member

👋 Hey folks! I have good news. We had an excellent open source contribution to this project that has implemented the checks input option.

All details about this new input option can be found in the following release -> v9.1.0.

Since it has been quite a while since this issue was first opened, I will go ahead and ping (@ mention) a few people that seemed interested in this feature.

@hfaulds @tiagonbotelho @metaory @KevinMSampson @pauleustice @redoz @bzurkowski

Cheers! 🎉

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

Successfully merging a pull request may close this issue.

2 participants