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

Check that the bottom PR is mergeable #20

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Conversation

AmaranthineCodices
Copy link
Collaborator

GitHub lets you check if a PR is actually mergeable via the gh CLI by
retrieving the mergeStateStatus field. This field has a bunch of
values (see the [API documentation][1] linked below), but the one we care
about is CLEAN, indicating that the PR has no merge conflicts and has
passed PR checks.

I'd like to eventually add this information to stack-pr view, but it will
require some extra piping, and will slow down the command due to the
extra network request.

1: https://docs.github.com/en/graphql/reference/enums#mergestatestatus

GitHub lets you check if a PR is actually mergeable via the `gh` CLI by
retrieving the `mergeStateStatus` field. This field has a bunch of
values (see the [API documentation][1] linked below), but the one we care
about is `CLEAN`, indicating that the PR has no merge conflicts and has
passed PR checks.

I'd like to eventually add this information to `stack-pr view`, but it will
require some extra piping, and will slow down the command due to the
extra network request.

1: https://docs.github.com/en/graphql/reference/enums#mergestatestatus
Copy link
Collaborator

@ZolotukhinM ZolotukhinM left a comment

Choose a reason for hiding this comment

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

LGTM % one minor remark!

@@ -424,7 +431,7 @@ def set_base_branches(st: List[StackEntry], target: str):

def verify(st: List[StackEntry], check_base: bool = False):
log(h("Verifying stack info"), level=1)
for e in st:
for index, e in enumerate(st):
Copy link
Collaborator

Choose a reason for hiding this comment

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

index doesn't seem to be used here, do we need this particular change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's used to test mergeability against only the bottom-most PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, sorry, my bad, I didn't realize the change below is in the same loop :)

@ZolotukhinM ZolotukhinM merged commit 804f0bf into main Aug 26, 2024
2 checks passed
@ZolotukhinM ZolotukhinM deleted the lbrown/report-mergeable branch August 26, 2024 17:56
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