workflows: small refactors #371216
workflows: small refactors #371216
Conversation
b54c977 to
648f47e
Compare
|
I added two more commits, one to rename some I would appreciate a thorough review of that README (@infinisil, @Mic92), both to improve its quality, but also to enhance / confirm my own understanding. |
This seems to be unused. It can be triggered manually, but is this really done? Is this superseded by the new eval checks or should we instead run this regularly?
All other workflows do - and most importantly actionlint only runs on .yml files!
mergedSha is available from needs.get-merge-commit, not needs.attrs. Actionlint rightfully complains about that. The code still works as expected because nixpkgs/ is checked out at mergedSha, so the diff will be between mergedSha and baseSha.
Same top-level ordering of keys / empty lines and same indentation for yaml lists. One blank line between each step. Makes it easier to read and compare the workflows.
648f47e to
7770521
Compare
There was a problem hiding this comment.
This is really nice, thank you!
The only feedback I have is that I don't think 27d5f29 should be done, because it could happen that the base branch changes between the different jobs. The original code "pinned" the base branch commit.
7770521 to
b32167a
Compare
The eval-aliases job is independent of attrs already.
No need for that limitation, which only artifically limits test-ability of CI in forks. Some other workflows like backports, cherry-pick checks and periodic merges are very specific to the release branches and don't need to be run in forks.
It seems odd to exclude PRs against release branches for those checks - especially when not excluding PRs against staging-** variants at the same time.
Less repetition, more consistency.
We currently use two different "base" commits, but the same name. One of them is the commit in which context the pull_request_target runs. The other is the parent of the merge commit. Those are **not** necessarily the same - see README introduced in the next commit for details. Renaming one of them for clarity. Since the pull_request_target related base commit is also called like that in GitHub Actions terminology, we rename the other. The best I could come up with is "target".
This introduces some basic concepts used in these workflows and a common terminology. At the same time we remove some of the comments from various workflow files, because they are assumed to be "general knowledge" through the README.
b32167a to
9ea7422
Compare
Ah, right. That is a left-over before I started to understand the nuances between what I then called "base commit" and "target commit" later. Good catch, thanks! Imho the "needs attrs" is still not required for the "Eval nixpkgs with aliases enabled" job, though - so I left that one line of this commit in there. Let me know if you think otherwise. |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-24.11
git worktree add -d .worktree/backport-371216-to-release-24.11 origin/release-24.11
cd .worktree/backport-371216-to-release-24.11
git switch --create backport-371216-to-release-24.11
git cherry-pick -x aa7335ca5f80f7d249bd7870d3d4cfd8ed79bb88 4d00c68aa42ef56c4be7a9692fdd5b035d134dfe fcb24b90d0704cd4511c9bb1b6be87f1e616bc17 72fd375d1c6685cf1e6cbaf1142f86fafc33103c 88afad8833aba51d5d24d2f667ab8eea2f9508cd 94c4c7bd3b886b7dcd408d01f60ad9964c6ef62b b64d5e1c0c6e2af9ddfc323866b7195d47a01256 58f8c536c66e6e6d62978885caf61d276df872d6 51b8ad219198913429c793511dff71c99e7979a0 ba09688dc88ac88a46a75412a1b72affcca16471 3e9f5c05eacaa83dab454c97b55c852f5337225e 9ea74225cc448f708c514ee03206323d732f122d |
|
Backport created in #372939. |
|
This seems to be giving me a boatload of e-mail spam on my Nixpkgs clone saying "Run failed: Periodic Merges (6h/24h)". For example: https://github.com/OPNA2608/nixpkgs/actions/runs/12726540788 |
While looking at and learning about the CI workflows, I ended up with some changes. Commit messages tell the story. Best reviewed by disabling whitespace changes in UI!
Relevant changes in behavior:
git fetch --shallow-since. Not sure about the exact value, 1 month is probably still a lot more than needed. But it runs in less than a minute instead of 6-7 minutes now.Other than that, cleanups, formatting, stuff like that.
More to come, but not in this PR.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.