Allow merging PRs with pending checks

[erickmendonca]

Hi,

Some repositories might have checks that never complete, e.g. CD checks that need authorization or new CI checks under testing.

I wrote some code to restrict the blockingChecks condition to only look for required checks in branch protection rules depending on a setting you can define in auto-merge.yml.

Would this be something we could add to probot-auto-merge?

[bobvanderlinden]

I missed this PR. But yes, I think this is certainly a good option. How do you do the configuration for this feature?

[erickmendonca]

I added a setting called something like “waitForBlockingChecksOnly” on auto-merge.yml. It’s working well. I’ll repackage it into a PR.

[bobvanderlinden]

Maybe we should have something like:

blockingChecks: all
blockingChecks: required
blockingChecks: none

blockingChecks: all is default, as that's the current behavior. I can imagine making it possible to specify the blocking checks for auto-merging specifically, but that's probably out of scope for this issue.

[danijeljw]

Has this been added yet? I just found out the hard way that my checks don't complete before the merge takes place and checks weren't validated on a merged branch 👎

[Tapchicoma]

@erickmendonca could you share code you've wrote? Having same issue that for some PRs some non-required checks may never finish

[erickmendonca]

@erickmendonca could you share code you've wrote? Having same issue that for some PRs some non-required checks may never finish

I am not sure if this is compatible with the current version of the tool, but this is what I did on my instance.

[rogerluan]

Maybe we should have something like:

blockingChecks: all
blockingChecks: required
blockingChecks: none

blockingChecks: all is default, as that's the current behavior. I can imagine making it possible to specify the blocking checks for auto-merging specifically, but that's probably out of scope for this issue.

I concluded, by testing, that the default is probably just the equivalent to blockingChecks: required, not all. Hope this helps someone, because this message led me to think there was an issue with my branch protection rules, while there wasn't 🙈 For more info on the issue I was having: #805

[sebastiankirsch]

@rogerluan My experience is that the default is indeed blockingChecks: all, since auto-merge doesn't merge a PR where required checks (as defined by a branch protection rule) are successful; yet other, irrelevant checks fail.
I'm no JS/TS expert, but https://github.com/bobvanderlinden/probot-auto-merge/blob/master/src/conditions/blockingChecks.ts gives the impression that all checks are considered.

[sebastiankirsch]

Maybe we should have something like:

blockingChecks: all
blockingChecks: required
blockingChecks: none

blockingChecks: all is default, as that's the current behavior. I can imagine making it possible to specify the blocking checks for auto-merging specifically, but that's probably out of scope for this issue.

@bobvanderlinden Seems to me that with requiredChecks in place, the required option is, well, not required, and thus a simple true/false configuration would be sufficient?

[rogerluan]

auto-merge doesn't merge a PR where required checks (as defined by a branch protection rule) are successful; yet other, irrelevant checks fail.

Yup, that's correct, and matches GitHub behavior. However, there's not only "success" and "failure" states - there's also "pending" (or inconclusive, or waiting to be reported etc). If it passes all required checks, and the other checks are NOT failed, it will merge. :)

[Borda]

@bobvanderlinden Seems to me that with requiredChecks in place, the required option is, well, not required, and thus a simple true/false configuration would be sufficient?

hello, is there a description for this requiredChecks I cannot find it in the readme...

[rogerluan]

hello, is there a description for this requiredChecks I cannot find it in the readme...

Seems like it's not documented 😬

[bobvanderlinden]

@Borda @rogerluan If you're referring to #312 (comment), that's hypothetical. It doesn't have an implementation (yet). requiredChecks itself is one of the conditions, but doesn't need any configuration atm.

[Borda]

@Borda @rogerluan If you're referring to #312 (comment), that's hypothetical. It doesn't have an implementation (yet). requiredChecks itself is one of the conditions, but doesn't need any configuration atm.

so is there a way-around ho to trigger only if all (or some named ones) pass?