First-class GitHub team reviews (reverted) by infinisil · Pull Request #456481 · NixOS/nixpkgs

infinisil · 2025-10-28T15:55:08Z

This changes automation to request reviews from listed GitHub teams instead of their individual members when possible, allowing the team maintainers to make use of GitHub's advanced code review settings and making the team-review-requested:NixOS/someTeam PR filter more useful.

This applies for:

lib.teams that are listed as a package maintainer using the new meta.teams (thanks to check-meta: add a teams attribute #394797), with lib.teams.*.github being set to the matching GitHub team:
```
meta = {
  teams = with lib.teams; [ someTeam ];
};
```
GitHub teams that are listed as a code owner in ci/OWNERS:
```
someFile @NixOS/someTeam
```

This pretty much implements #447514 after the preparatory work of #449986 and #450864.

Things done

Fully tested locally using various PRs that exercise all code paths

Add a 👍 reaction to pull requests you find important.

wolfgangwalther

I just skimmed the code quickly, the two biggest issues we need to sort out:

This introduces a lot of new bash code, which is against our policy on new CI code.
We will need to see how to deal with #450864, which currently changes the social contract of Nixpkgs massively, imho.

Also found a specific change that would currently break the labeler.

Blocking this PR, so that it's not accidentally merged.

ci/eval/compare/maintainers.nix

infinisil · 2025-10-29T19:16:41Z

We will need to see how to deal with lib.teams: Sync from GitHub (members/shortName/scope) #450864, which currently changes the social contract of Nixpkgs massively, imho.

I guess you mean #450864 (comment) (I suggest opening a new issue), but this PR just depends on the fact that GitHub teams have the same members as the linked lib.teams, so it won't be affected even if the process or syncing is flipped around in the future.

This introduces a lot of new bash code, which is against our policy on new CI code.

I already complained in #456341 (comment), but I concur that it makes sense for the policy as worded to apply here. I'm not very happy about it because most of the code was already written before the policy existed, and I assumed it wouldn't apply because no new scripts are created and all the review requesting code is written in bash right now.

How about this as a compromise: I'll try to rewrite the request-reviewers.sh script (which contains the most changes) in JS, but everything else will be left as bash for this PR.

infinisil · 2025-10-29T22:46:56Z

@wolfgangwalther I now committed a working JS version of the script as mentioned above, but can you help me integrate it? In particular I'd just like to have a buildable derivation for ci/github-script that produces a runnable result/bin/github-script executable for the run script. This will make it very easy to swap out calls to request-reviewers.sh.

wolfgangwalther · 2025-11-01T10:40:47Z

I now committed a working JS version of the script as mentioned above, but can you help me integrate it? In particular I'd just like to have a buildable derivation for ci/github-script that produces a runnable result/bin/github-script executable for the run script. This will make it very easy to swap out calls to request-reviewers.sh.

I believe that a "runnable github-script" thing is fundamentally the wrong thing. We'll want to run this with actions/github-script - the local wrapper is just something for development, but we don't even try to replicate the upstream environment 100%.

I didn't look at the script you wrote in full detail, yet, but two notes:

I'm pretty sure that ci/request-reviews: untangle owner-related bash code #457503 should help a lot with the integration problems you're facing. I'll likely add 1-2 more commits.
My idea for migrating the reviewers stuff to JavaScript is to integrate it into labels.js (soon to be renamed to bot.js or something). This will give us a few benefits for requesting reviews in general, such as being able to expire review requests, rotating through them over time, requesting reviews for already existing PRs after becoming owner/maintainer and so on. I have yet to think about how this interacts with this PR here, aka in which order to do things to make it easier overall. Please give me a few days to do so.

infinisil · 2026-01-25T13:55:57Z

@MattSturgeon Thanks for the review and pointers! I knew how code could be run locally, though was originally set on testing it in a real repo (like I've done before on CI changes). However it turns out this isn't easily possible anymore due to the merge queue and maintainer maps.

Anyways, I now just tested the code locally to cover as many paths as possible. In particular I used #471082 to test the "PR reviews on behalf of a team" thing. I fixed some minor problems while doing so, and refactored the code a bit for clarity.

I'm 95% confident this is going to work in practice now and am ready to do follow-ups if it doesn't. @MattSturgeon want to take another look or give an approve/merge? :D

philiptaron · 2026-01-31T15:40:42Z

Matt (or any other member of CI team) is free to merge without this, but I plan to review/test and merge today or tomorrow, personal and family bandwidth permitting.

philiptaron

Thanks for working on this! The approach looks good, but I found a few bugs while reviewing.

I ran TypeScript with --checkJs on the JS files and it caught these issues. I will be looking into setting up automatic type checking for ci/github-script/ since it catches problems like this quickly.

ci/github-script/reviewers.js

infinisil · 2026-02-03T13:24:46Z

@philiptaron Thank you very much, fixed now!

philiptaron · 2026-02-03T18:00:44Z

ci/github-script/bot.js

+        .request({
+          method: 'GET',
+          url: '/organizations/{orgId}/team/{id}',
+          orgId: github.repository_owner_id,


I can't find this anywhere. Is this context.payload.repository.owner.id?

Ohh nice catch, yeah I took this from https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#github-context, but right, that github variable is different from this one.

While context.payload.repository.owner.id doesn't exist, context.payload.pull_request.base.user.id does, now using that! Tested locally

philiptaron

Requesting changes for the github.repository_owner_id thing. I didn't test this locally yet, but I can't find anything in Octokit that would set this.

Co-Authored-By: Alexander Bantyev <[email protected]>

nixpkgs-ci · 2026-02-06T01:50:21Z

Successfully created backport PR for release-25.11:

[Backport release-25.11] First-class GitHub team reviews #487491

mdaniels5757 · 2026-02-06T03:09:57Z

We had to revert in #487507, unfortunately. Please see there for details.

infinisil · 2026-02-07T13:00:56Z

Unfortunate, reopened with the fix as #487955

infinisil mentioned this pull request Oct 28, 2025

[Reverted] First-class GitHub team reviews #456341

Merged

This comment was marked as resolved.

Sign in to view

github-project-automation bot added this to Stdenv Oct 28, 2025

infinisil force-pushed the first-class-team-reviews branch from eba5e54 to 7e4ef3c Compare October 28, 2025 20:45

nixpkgs-ci bot added 6.topic: policy discussion Discuss policies to work in and around Nixpkgs backport release-25.05 labels Oct 28, 2025

nix-owners bot requested review from Artturin, Ericson2314, MattSturgeon, Mic92, RossComputerGuy, emilazy, philiptaron, reckenrode, wolfgangwalther and zowoq October 28, 2025 20:53

wolfgangwalther previously requested changes Oct 29, 2025

View reviewed changes

ci/eval/compare/maintainers.nix Show resolved Hide resolved

github-project-automation bot moved this to In Progress in Stdenv Oct 29, 2025

infinisil force-pushed the first-class-team-reviews branch from f8f8963 to 3839545 Compare October 31, 2025 22:49

infinisil mentioned this pull request Nov 1, 2025

First-class GitHub team support for review requests #447514

Closed

infinisil linked an issue Nov 1, 2025 that may be closed by this pull request

First-class GitHub team support for review requests #447514

Closed

wolfgangwalther mentioned this pull request Nov 1, 2025

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

Merged

2 tasks

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 1, 2025

nixpkgs-ci bot requested a review from Mic92 January 22, 2026 15:40

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 22, 2026

infinisil force-pushed the first-class-team-reviews branch from 780112a to d53c754 Compare January 25, 2026 11:26

nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 25, 2026

infinisil force-pushed the first-class-team-reviews branch from d53c754 to e9c544a Compare January 25, 2026 13:44

philiptaron requested changes Feb 1, 2026

View reviewed changes

ci/github-script/reviewers.js Show resolved Hide resolved

ci/github-script/reviewers.js Outdated Show resolved Hide resolved

ci/github-script/reviewers.js Outdated Show resolved Hide resolved

philiptaron mentioned this pull request Feb 1, 2026

ci: typecheck github-script code #485846

Open

13 tasks

infinisil force-pushed the first-class-team-reviews branch from e9c544a to d40dd99 Compare February 3, 2026 13:24

infinisil requested a review from philiptaron February 3, 2026 13:24

philiptaron reviewed Feb 3, 2026

View reviewed changes

philiptaron requested changes Feb 3, 2026

View reviewed changes

infinisil force-pushed the first-class-team-reviews branch from d40dd99 to a26b6e7 Compare February 5, 2026 13:50

ci: First-class team package maintainer review requests

193deb8

Co-Authored-By: Alexander Bantyev <[email protected]>

infinisil force-pushed the first-class-team-reviews branch from a26b6e7 to 193deb8 Compare February 5, 2026 13:52

philiptaron approved these changes Feb 6, 2026

View reviewed changes

philiptaron added this pull request to the merge queue Feb 6, 2026

Merged via the queue into NixOS:master with commit ba626b8 Feb 6, 2026
48 of 51 checks passed

github-project-automation bot moved this from In Progress to Done in Stdenv Feb 6, 2026

nixpkgs-ci bot mentioned this pull request Feb 6, 2026

[Backport release-25.11] First-class GitHub team reviews #487491

Closed

github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Feb 6, 2026

mdaniels5757 mentioned this pull request Feb 6, 2026

Revert "First-class GitHub team reviews" #487507

Merged

infinisil mentioned this pull request Feb 7, 2026

First class team reviews (try #2) #487955

Merged

infinisil changed the title ~~First-class GitHub team reviews~~ First-class GitHub team reviews (reverted) Feb 7, 2026

Uh oh!

Conversation

infinisil commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

This comment was marked as resolved.

wolfgangwalther left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

infinisil commented Oct 29, 2025

Uh oh!

infinisil commented Oct 29, 2025

Uh oh!

wolfgangwalther commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

infinisil commented Jan 25, 2026

Uh oh!

philiptaron commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

infinisil commented Feb 3, 2026

Uh oh!

philiptaron Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

infinisil Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nixpkgs-ci bot commented Feb 6, 2026

Uh oh!

mdaniels5757 commented Feb 6, 2026

Uh oh!

infinisil commented Feb 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

infinisil commented Oct 28, 2025 •

edited

Loading

wolfgangwalther commented Nov 1, 2025 •

edited

Loading

philiptaron commented Jan 31, 2026 •

edited

Loading

philiptaron left a comment •

edited

Loading

infinisil Feb 5, 2026 •

edited

Loading