Skip to content

[Backport release-25.05] workflows/labels: manage approval labels#416410

Merged
philiptaron merged 7 commits intorelease-25.05from
backport-415259-to-release-25.05
Jun 14, 2025
Merged

[Backport release-25.05] workflows/labels: manage approval labels#416410
philiptaron merged 7 commits intorelease-25.05from
backport-415259-to-release-25.05

Conversation

@nixpkgs-ci
Copy link
Contributor

@nixpkgs-ci nixpkgs-ci bot commented Jun 13, 2025

Bot-based backport to release-25.05, triggered by a label in #415259.

  • Before merging, ensure that this backport is acceptable for the release.
    • Even as a non-committer, if you find that it is not acceptable, leave a comment.

@github-actions github-actions bot added 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 4.workflow: backport This targets a stable branch 6.topic: policy discussion Discuss policies to work in and around Nixpkgs labels Jun 13, 2025
@github-actions github-actions 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. labels Jun 13, 2025
@wolfgangwalther
Copy link
Contributor

Backport on hold until we have looked into #415259 (comment) a bit more.

This brings back the "minimize CI reviews after dismissal" job that was
previously removed. The first time around, we had a single job triggered
by the `pull_request_review` event. This lacks permission to do
meaningful stuff, though.

This time, we trigger an empty no-op job on `pull_request_review` and
then run a second workflow on `workflow_run`. This can run with the
proper permissions.

(cherry picked from commit a34a22d)
This moves the actual labeling from the eval workflow to the labels
workflow. At this stage, this only has a disadvantage: Adding the
topic-labels to the pull request will now only happen after eval has
finished, instead of instantly.

We will only benefit from this later, when we manage approval related
events. With this change, we will have the comparison results and thus
the package maintainer info available.

(cherry picked from commit 2d0bcd7)
The category 12 labels for number of approvals and approved by package
maintainer are now automatically managed by the labels workflow. The
logic is slightly different from the "by: package-maintainer" label. For
approval, it's enough if *any one* maintainer approves the PR to have
the label added, even if the PR touches multiple packages.

The workflow only counts approved reviews, no matter whether there had
been a push in the meantime or not. To achieve the currently used logic
of "expiring approvals after push", we will have to set up a branch
protection rule, which actually dismissed those reviews automatically.

(cherry picked from commit 5f09e16)
This can happen when two PRs run at the same time, which come from
different forks, but have the same head branch name.

github.head_ref is suggested by GitHub's docs, but.. that's not really
useful for cases with forks.

(cherry picked from commit 7ba7720)
…lows

This didn't work as intended. When a workflow is run with
`workflow_call`, it will have `github.workflow` set to the *parent*
workflow. So the `caller` input that we passed, resulted in this
concurrency key:

```
Eval-Eval-...
```

But that's bad, because the labels and reviewers workflows will cancel
each other!

What we actually want is this:
- Label and Reviewers workflow should have different groups.
- Reviewers called via Eval and called directly via undraft should have
*different* groups.

We can't use the default condition we use everywhere else, because
`github.workflow` is the same for Label and Reviewers. Thus, we hardcode
the workflow's name as well. This essentially means we have this as a
key:

```
<name-of-running-workflow>-<name-of-triggering-workflow>-<name-of-event>-<name-of-head-branch>
```

This should do what we want.

Since workflows can be made reusable workflows later on, we add those
hardcoded names to *all* concurrency groups. This avoids copy&paste
errors later on.

(cherry picked from commit 6793e23)
@wolfgangwalther wolfgangwalther force-pushed the backport-415259-to-release-25.05 branch from d4787fe to 4aa51a9 Compare June 14, 2025 12:27
@nix-owners nix-owners bot requested a review from philiptaron June 14, 2025 12:29
A single reviewer approving a Pull Request multiple times should only
count once.

(cherry picked from commit 2e03351)
This currently fails with:

```
Method Set.prototype.has called on incompatible receiver undefined
```

Seems like my syntax test previously only hit the case without
maintainers, in which case it doesn't throw :/.

(cherry picked from commit 4b9fb45)
@github-actions github-actions bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 14, 2025
@philiptaron philiptaron merged commit 9d34493 into release-25.05 Jun 14, 2025
24 of 27 checks passed
@philiptaron philiptaron deleted the backport-415259-to-release-25.05 branch June 14, 2025 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.workflow: backport This targets a stable branch 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 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