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.

Sign up for GitHub

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

Jump to bottom

Improve PR merge check for CI #347786

Merged
merged 3 commits into from
Oct 12, 2024
Merged

Improve PR merge check for CI #347786

merged 3 commits into from
Oct 12, 2024
+126 −39

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Oct 10, 2024 •
edited
Loading

The Nixpkgs-vet workflow has been using a script to check whether the PR to check is mergeable and abort the check if it's not. This PR improves the script by:

  • Making it reusable for other workflows. It can be used for any workflow checking out the PR really.
  • Handle PRs that are not open anymore, fixing this kind of misleading error: 
    Checking whether the pull request can be merged
GitHub is still computing whether this PR can be merged, waiting 5 seconds before trying again (4 retries left)
[...]
GitHub is still computing whether this PR can be merged, waiting 80 seconds before trying again (0 retries left)
Checking whether the pull request can be merged
Not retrying anymore. It's likely that GitHub is having internal issues: check https://www.githubstatus.com/
  • Write some documentation for this script and how it can be used in workflows

Things done

This work is sponsored by Antithesis ✨

Add a 👍 reaction to pull requests you find important.

philiptaron
philiptaron approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Approved with a couple comments.

ci/get-merge-commit.sh Show resolved Hide resolved
ci/get-merge-commit.sh Outdated
fi

mergeable=$(jq -r .mergeable <<< "$prInfo")
mergedSha=$(jq -r .merge_commit_sha <<< "$prInfo")
Copy link
Contributor

Choose a reason for hiding this comment

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

Want to do this after testing for mergable?

@infinisil
workflows/nixpkgs-vet: Make merge check script reusable
7f9d297 
This is useful for other workflows as well. Originally I thought it
couldn't be put in the repo, but it can (just needs another checkout)
@infinisil
ci/get-merge-commit.sh: Exit for non-open PRs
048f4aa 
If PRs aren't open (either merged or closed), GitHub never computes
whether the PR is mergeable, so we'd wait forever, which has been
happening:
https://github.com/NixOS/nixpkgs/actions/runs/11279197077/job/31369348101#step:2:59
@infinisil
ci/get-merge-commit.sh: Add documentation
e6a8855 
And distinguish exit codes
@infinisil
Copy link
Member Author

@philiptaron Thanks, addressed!

@philiptaron philiptaron merged commit d6d9c61 into NixOS:master Oct 12, 2024
23 checks passed
@infinisil infinisil deleted the improved-ci-merge-check branch October 12, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philiptaron philiptaron philiptaron approved these changes

@LeSuisse LeSuisse Awaiting requested review from LeSuisse

@mweinelt mweinelt Awaiting requested review from mweinelt

@risicle risicle Awaiting requested review from risicle

Assignees
No one assigned
Labels
6.topic: policy discussion 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@infinisil @philiptaron