hrvey / combine-prs-workflow

Combine/group together PRs (for example from Dependabot and similar services)
https://www.hrvey.com/blog/combine-dependabot-prs
MIT License
296 stars 51 forks source link

Error on getting status #17

Open BravoNatalie opened 1 year ago

BravoNatalie commented 1 year ago

I'm having an issue with the getting status.

image
BravoNatalie commented 1 year ago

This behavior happens if the PR does not have any checkers, so to get through this issue I added the mergeable property into the stateQuery.

const stateQuery = `query($owner: String!, $repo: String!, $pull_number: Int!) {
                                  repository(owner: $owner, name: $repo) {
                                    pullRequest(number:$pull_number) {
                                      mergeable
                                      commits(last: 1) {
                                        nodes {
                                          commit {
                                            statusCheckRollup {
                                              state
                                            }
                                          }
                                        }
                                      }
                                    }
                                  }
                                }`
const vars = {
  owner: context.repo.owner,
  repo: context.repo.repo,
  pull_number: pull['number']
};

const result = await github.graphql(stateQuery, vars);
const [{ commit }] = result.repository.pullRequest.commits.nodes;

if(commit.statusCheckRollup != null){
  const state = commit.statusCheckRollup.state
  console.log('Validating status: ' + state);
  if(state != 'SUCCESS') {
    console.log('Discarding ' + branch + ' with status ' + state);
    statusOK = false;
  }
} else {
  const mergeable = result.repository.pullRequest.mergeable
  console.log('PR does not have statusCheckRollup, but mergeability is: ' + mergeable);
  if(mergeable !== 'MERGEABLE'){
    console.log('Discarding ' + branch + ' with mergeability: ' + mergeable);
    statusOK = false;
  }
}
martingjaldbaek commented 1 year ago

Hi @BravoNatalie - thanks for the PR!

And sorry this took some time to look at - first off, life happened. But secondly I also wasn't sure if this was strictly an improvement, or if there was a risk of causing problems/confusion, so I've been mulling this over. Here are the issues I've been considering:

1) There is already the option of setting "mustBeGreen" to false, as a way to work with PRs that don't get checks run on them

Overall, if it wasn't for 3) I would have merged this already. But I do worry that having the workflow be able to silently fail for non-obvious reasons is a bit of a foot-gun for users. Any thoughts?

bruteforks commented 1 year ago

maybe specifying what permissions need to be applied could help. I'm getting this same error after trying to fix this error: "[HttpError]: Resource not accessible by integration"