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

Allows automerging PRs in the 'unstable' state #214

Merged
merged 1 commit into from
Jan 14, 2021

Conversation

ryanmoran
Copy link
Member

Summary

As outlined in an issue on octokit, the unstable state is defined as "Failing/pending commit status that is not part of the required status checks. Merging is allowed."

Use Cases

Running the "Auto Merge" workflow itself induces an unstable state as it is a pending check that is not part of the required status checks. This is different than the blocked state which is induced by running one of the required status checks. Either way, a merge will only be possible if all of the required checks have passed and so it should be safe to allow the "Auto Merge" workflow to attempt to merge the PR given an unstable state.

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have added an integration test, if necessary.

As outline in an [issue on
octokit](octokit/octokit.net#1763), the
`unstable` state is defined as "Failing/pending commit status that is
not part of the required status checks. Merging is allowed."

Running the "Auto Merge" workflow itself induces an `unstable` state as
it is a pending check that is not part of the required status checks.
This is different than the `blocked` state which is induced by running
one of the required status checks. Either way, a merge will only be
possible if all of the required checks have passed and so it should be
safe to allow the "Auto Merge" workflow to attempt to merge the PR given
an `unstable` state.
@ryanmoran ryanmoran requested a review from a team as a code owner January 14, 2021 18:09
@@ -30,7 +30,7 @@ jobs:
echo "::set-output name=mergeable_state::$(echo "${payload}" | jq -r -c .mergeable_state)"

- name: Merge
if: ${{ steps.pull_request.outputs.mergeable_state == 'clean' }}
if: ${{ steps.pull_request.outputs.mergeable_state == 'clean' || steps.pull_request.outputs.mergeable_state == 'unstable' }}
Copy link
Member

@sophiewigmore sophiewigmore Jan 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there cases where 'clean' can still happen? From your description it seems like it would always be in the 'unstable' state since at this point the auto-merge workflow would be running.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue here is that the GitHub API is not consistent in what it returns. I believe there is some sort of race condition wherein sometimes it will return clean and other times it returns unstable. Both should be acceptable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh ok that's fair then

@sophiewigmore sophiewigmore merged commit 94af1d7 into main Jan 14, 2021
@sophiewigmore sophiewigmore deleted the allow-unstable-merge branch January 14, 2021 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants