Announce in Slack if automerge fails because PR is not mergeable #1873
Conversation
botify
requested review from
Gonals and
thienlnam
and removed request for
a team
| @@ -26,6 +26,28 @@ jobs: | |||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |||
| PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} | |||
|
|
|||
| # This Slack step is duplicated in all workflows, if you make a change to this step, make sure to update all | |||
There was a problem hiding this comment.
Should we remove all the other instances? Can we exit the job here as well?
There was a problem hiding this comment.
Rearranged so we fail the workflow then this announce step happens. However, I think we should leave the other announce steps in case the 3rd-party auto-approve and auto-merge actions fail for some other reason we're not aware of.
.github/workflows/automerge.yml
Outdated
| @@ -26,6 +26,31 @@ jobs: | |||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |||
| PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} | |||
|
|
|||
| - name: Fail workflow if PR is not mergeable | |||
| if: ${{ steps.isPullRequestMergeable.outputs.IS_MERGEABLE }} | |||
There was a problem hiding this comment.
Should this be ?
| if: ${{ steps.isPullRequestMergeable.outputs.IS_MERGEABLE }} | |
| if: ${{ steps.isPullRequestMergeable.outputs.IS_MERGEABLE == 'false' }} |
Also do you know if this exit 1 will stop all the other jobs from running and so therefore we don't need the if: checks for == 'true'?
There was a problem hiding this comment.
Fixed the conditional!
Also do you know if this exit 1 will stop all the other jobs from running and so therefore we don't need the if: checks for == 'true'?
Yeah, I think this is true.
|
Merging to get deploys working! |
|
@AndrewGable looks like this was merged without passing tests. Please add a note explaining why this was done or remove the |
🎉 |
Fixing failed workflow run: https://github.com/Expensify/Expensify.cash/runs/2143920797?check_suite_focus=true
The
mergeable_stateof a PR is mix of whether it ismergeableand has all its PR checks completed and passed. While checks are still running, themergeable_stateisunstable. Bothmergeableandmergeable_stateare determined asynchronously in Github using background jobs.So we have a custom Javascript action to poll continually until the mergeability of a PR is settled. This PR adds a slack announcement in the case that an
automergePR is created that's not mergeable (should not happen, hence the slack announcement). We also could poll and wait for themergeable_stateto be settled, but frankly we don't care about that field, since we skip all the checks on theautomergePRs anyways. So this PR also removes the checks we had in place formergeable_state, which caused the automerge workflow to silently fail unnecessarily.