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

pr status: fix checking branch protection rules on the base branch #2988

Merged
merged 1 commit into from Feb 17, 2021

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Feb 17, 2021

Instead of checking branch protection rules on the main branch of the repository, branch protection rules for a specific PR should be checked on its base branch, since not all PRs are based on the main branch.

Additionally, do not display "Up to date" if the actual merge status reported from the server was "UNKNOWN" or "DIRTY", since in those cases "Up to date" could be false information.

Followup to #2952 /cc @redreceipt

Instead of checking branch protection rules on the main branch of the
repository, branch protection rules for a specific PR should be checked
on its base branch, since not all PRs are based on the main branch.

Additionally, do not display "Up to date" if the actual merge status
reported from the server was "UNKNOWN" or "DIRTY", since in those cases
"Up to date" could be false information.
@mislav mislav requested a review from a team February 17, 2021 17:05
@mislav mislav added this to Backlog 🗒 in The GitHub CLI via automation Feb 17, 2021
@mislav mislav moved this from Backlog 🗒 to Needs review 🤔 in The GitHub CLI Feb 17, 2021
@redreceipt
Copy link
Contributor

yes! thank you for that. much better. thanks for being so open to contributions in the first place :)

The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Feb 17, 2021
@mislav mislav merged commit 70d4786 into trunk Feb 17, 2021
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Feb 17, 2021
@mislav mislav deleted the strict-status-checks-base branch February 17, 2021 17:13
The GitHub CLI automation moved this from Pending Release 🥚 to Needs to be merged 🎉 Feb 17, 2021
Copy link
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

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

Nice catch.

@redreceipt
Copy link
Contributor

What's the release flow? How/when does this make it into a stable release?

@mislav
Copy link
Contributor Author

mislav commented Feb 17, 2021

@redreceipt There will be a release that includes this soon.

@vilmibm vilmibm moved this from Needs to be merged 🎉 to Pending Release 🥚 in The GitHub CLI Feb 17, 2021
@github-actions github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
The GitHub CLI
  
Done 💤
Development

Successfully merging this pull request may close these issues.

None yet

4 participants