Skip to content

ci/request-reviews: untangle owner-related bash code#457503

Merged
wolfgangwalther merged 4 commits intoNixOS:masterfrom
wolfgangwalther:ci-owners-refactor
Nov 2, 2025
Merged

ci/request-reviews: untangle owner-related bash code#457503
wolfgangwalther merged 4 commits intoNixOS:masterfrom
wolfgangwalther:ci-owners-refactor

Conversation

@wolfgangwalther
Copy link
Contributor

@wolfgangwalther wolfgangwalther commented Nov 1, 2025

Four refactors to untangle the various scripts in ci/request-reviews. This should help with #456481 (comment).

Things done


Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs backport release-25.05 labels Nov 1, 2025
Instead of requesting owners and maintainer separately, each with their
own limit of 10 review requests, we now run this together. This unties
the logic and allows easier refactoring. Also, it gives us a consistent
threshold of when not to request reviews anymore, which I set to 15.
Before, this could have been anything between 10 and 20, depending on
how the reviewers distributed over owners and maintainers.
This is just a refactor, no functional change. It is a preparation for a
future change, where `get-code-owners.sh` can be moved entirely into
eval/compare. This can only happen once we removed the remaining `gh
api` calls from it.
All the github related logic is now bundled in `request-reviewers.sh`.
This allows moving the `get-code-owners.sh` file into the eval/compare
step in the next commit.
This moves the parsing of ci/OWNERS into the Nix sandbox. We also get
rid of checking out the nixpkgs repo another time in the reviewers
workflow - we already have everything we need in the eval/compare job.

The creation of owners.txt in this way is only temporary, it should
eventually be moved further, similar to how maintainers.json is
currently migrating to a maintainer map for the whole repo stored on the
target branch as artifact.
@wolfgangwalther wolfgangwalther marked this pull request as ready for review November 1, 2025 14:12
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

Can't spot any obvious issues.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 2, 2025
@wolfgangwalther wolfgangwalther added this pull request to the merge queue Nov 2, 2025
@wolfgangwalther
Copy link
Contributor Author

(I think this will make the reviewers job fail when it is triggered when a PR is undrafted, but no other Eval run happens - I think these cases will not happen to often, though, and I can watch out for them in the Actions tab easily)

Merged via the queue into NixOS:master with commit 1774ef8 Nov 2, 2025
53 of 55 checks passed
@wolfgangwalther wolfgangwalther deleted the ci-owners-refactor branch November 2, 2025 15:44
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Nov 2, 2025

Successfully created backport PR for release-25.05:

@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Nov 2, 2025
@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Nov 2, 2025

I see jobs failing like this currently:

/nix/store/2yw28m2xvw8mllrk638l99p4bk1qnbv0-request-reviews/bin/.request-reviewers.sh-wrapped: line 62: entry: unbound variable

Should be fixed in #457838.

@wolfgangwalther
Copy link
Contributor Author

wolfgangwalther commented Nov 2, 2025

And one more for PRs without relevant code owners:

cat: comparison/owners.txt: No such file or directory

Fix in #457843

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants