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

Use /check-runs instead of /status to support repos using GitHub Actions for CI #6

Closed joelittlejohn closed 2 years ago

joelittlejohn commented 3 years ago

Hi, thanks for your work to create this workflow! I was thinking the same way after seeing dependabot raise 6 PRs at a time, and I was delighted to find your blog post.

When I initially installed this workflow, it failed to select any PRs to combine. I noticed that I would always see Validating status: pending in the log for every PR, even if the PR had successfully passed all the build and test checks that I use.

I did a little googling and found this:

https://github.community/t/state-is-pending-statuses-are-missing-with-github-api-v3/14386

Which I think means that if people use GitHub actions to build/test their repo then the /status endpoint you're using in this workflow will never show the commit state as 'success'. I found I could fix the problem by switching to use the /check-runs endpoint and checking the 'conclusion' of each check run in the same way.

I'm not sure if this change is compatible with external CI systems that are not GitHub actions, so I understand if you don't want to merge this. I just wanted to raise the PR and share my code so that others can use this modification if they want to. I've tested this and the results look good to me.

I'm interested to know though, when you built this workflow were you using something like Travis to check PRs?

martingjaldbaek commented 3 years ago

Thanks for sharing this. I'll test whether this also works for external CI (I'm using the workflow with CircleCI myself). But even if it isn't, it could still be a good idea to add a config option whether to use /status or /check-runs, so it's easily available to those that need it. There is already one called mustBeGreen, so it wouldn't be much hassle to replace that with an "enum" where users could choose between "status", "checks" or "none". Anyways, I'll see if that turns out to be necessary, and take care of the rest :)

martingjaldbaek commented 3 years ago

Hey, sorry for being a bit slow with this :) I've tested the PR, but unfortunately it doesn't work with external CI out of the box - so it seems like there's a bit more work in creating a hybrid solution where it can work both with external CI and CI based on GitHub Actions at the same time. But for now your PR gives those who need it an easy to discover alternative :)

martingjaldbaek commented 2 years ago

Hey all. I just merged a new update to the master branch. The big new thing is that the workflow can gracefully recover from merge conflicts (it just merges as many PRs as it can, and lists which ones failed).

But at the same time I also updated it to use the new "Combined Status API" to check which PRs are green. I've read some indications that this new API might unify the data from the two approaches to checking the PR status (the old Statuses API and the new Check Runs API). So I was wondering if anyone using Github CI/Check Runs could give the latest version on master a try, and report back if it works?

Freubert commented 2 years ago

@martingjaldbaek this doesn't seem to work yet. The API still reports back status pending when using Github CI.

martingjaldbaek commented 2 years ago

@Freubert Thanks a lot for checking that! I dug into it some more, and it turns out the API call that I thought was a combined "check runs" and "statuses" API isn't actually that. Such an API does apparently exist, but it's not available from GitHub's REST API, but it is available from their GraphQL API - so I switched to using that instead. I've verified that this works with external CI, but could you (or someone else using GitHub Action for CI) check the latest master version, and tell me if it correctly detects the CI status for you as well?

Freubert commented 2 years ago

@martingjaldbaek I can confirm that the current master state now works for Github CI. Thanks for fixing!

martingjaldbaek commented 2 years ago

@Freubert Thanks for checking it (again) and reporting back. Since the current master branch now has a version that should be strictly superior to this PR (since it works with both GitHub's own CI and external CI at the same time), I'll go ahead and close this PR.