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

[release-24.05] Mass pings be gone #348642

Merged
merged 18 commits into from
Oct 24, 2024
Merged

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Oct 14, 2024

roberth and others added 18 commits October 15, 2024 00:28
This allows shell.nix to be run with the latest tools instead of
the pinned ones when desired, which is probably not very often,
but useful nonetheless.

(cherry picked from commit b07c441)
Allows reusing it in more places

(cherry picked from commit 1700d00)
Also post a comment in case base branch is wrong
  This guides newcomers in how to smoothly handle the potentially scary
  situation of having thousands of commits listed in a PR.

  While CI shows the same, people might not even look at CI if the PR
  looks botched.

(cherry picked from commit 369cfa0)
(cherry picked from commit 5695bf6)
(cherry picked from commit 87a2986)
(cherry picked from commit 93dcd42)
Since NixOS#336261 we have CI that
checks that the codeowners file is valid:

https://github.com/NixOS/nixpkgs/actions/runs/11243668280/job/31260095472#step:7:34

Which files are correct (or whether they were removed) was determined
using the Git history and some grepping

(cherry picked from commit dd28082)
Apparently it started requesting reviews from code owners already
because the DRY_MODE from the global env was overridden in the local job
declaration: NixOS#347354 (comment)

(cherry picked from commit c1710f2)
The automation should never rerequest reviews from users that already
reviewed the changes, which is what was happening before this change:

NixOS#347354 (comment)

Also reorder the arguments to make more sense

(cherry picked from commit 9a054bb)
This makes this codeowner mechanism behave differently than the native
one, but there's no other way to avoid rerequesting reviews from teams
when a member already reviewed the PR.

(cherry picked from commit 1ff83b2)
This effectively disables the native GitHub codeowners feature
and enables the new alternate codeowners mechanism introduced in
NixOS#336261

This means that:
- We can now declare users without write access as code owners!
- Targeting the wrong branch won't trigger mass pings anymore!

(cherry-picked from commit b01ca00)
This is not a problem anymore with the parent commit

(cherry picked from commit c246403)
…arent

Also, fix the description of the text

(cherry picked from commit c721e91)
The codeowner-validator build declared in ci/codeowners-validator was
not cached before and needed to be built for every PR, which is slow and
wasteful: https://github.com/NixOS/nixpkgs/actions/runs/11280533037/job/31373720922

(cherry picked from commit f9b28d5)
Turns out if :<something> is passed, a local branch is updated, which
can conflict if the PR branch starts with "pr". I tried to avoid that
with the original code but apparently that didn't work!

https://github.com/NixOS/nixpkgs/actions/runs/11284183639/job/31384967152?pr=347822

    Fetching the PR commit history
    From https://github.com/linj-fork/nixpkgs
     * [new branch]            pr/kanata-add-version-check -> fork/pr
    error: cannot lock ref 'refs/remotes/fork/pr/kanata-add-version-check': 'refs/remotes/fork/pr' exists; cannot create 'refs/remotes/fork/pr/kanata-add-version-check'
     ! [new branch]            pr/kanata-add-version-check -> fork/pr/kanata-add-version-check  (unable to update local ref)
    error: some local refs could not be updated; try running

(cherry picked from commit 299a181)
Previously, headRef points to the master branch of Nixpkgs, which
basically means no code owner review will be requested.

The problem can be verified using the following command.

$ DRY_MODE=1 ./ci/request-reviews/request-reviews.sh NixOS/nixpkgs 347973 ci/OWNERS
[...]
This PR touches 0 files
Requesting reviews from: {
  "reviewers": []
}
[...]

Additionally, the comment about conflicts is removed thanks to the
unambiguous way of specifying ref.

(cherry picked from commit f4c6e11)
It's possible to have different casings in OWNERS, so we need to handle
that

(cherry picked from commit e612b89)
@infinisil
Copy link
Member Author

infinisil commented Oct 14, 2024

Cherry-pick check can be ignored because it pretty much just fails because b01ca00 had to be manually backported, since it moves the CODEOWNERS file, which differs between master and release-24.05

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Oct 15, 2024
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.

Works for me, though the age of 24.05 is quickly coming to an end.

@Mic92 Mic92 merged commit f4602b7 into NixOS:release-24.05 Oct 24, 2024
25 of 26 checks passed
philiptaron pushed a commit that referenced this pull request Oct 26, 2024
Was removed in 7e73ead, but only
started failing once it actually started getting checked with #348642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: policy discussion 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants