-
Notifications
You must be signed in to change notification settings - Fork 27
Don't require status checks for staging #139
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
Conversation
|
I think we should go a different direction: We should allow nixpkgs-ci (which handles those merges) to bypass, too - and keep the feature enabled for staging. |
|
Looking up the integration's type and ID etc. is tedious - it's probably simplest, if you just add the bypasser and then export, @infinisil. |
|
Note that periodic merges into Please be careful here; the I’d recommend hopping into the Staging room on Matrix to discuss the workflows involved and the issues we often have with CI if you don’t want to be conservative about changes that apply to those branches. |
Well, that's precisely what I concluded in #130 (comment) as well - every committer needs to be able to bypass these checks for staging-* branches. We just forgot about the |
| "refs/heads/release*", | ||
| "refs/heads/staging*", | ||
| "refs/heads/haskell-updates" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that if the conclusion is you'd rather want to go disable this entirely for staging - you'd also need to do it for haskell-updates, because the periodic merges in there are still broken right now.
https://github.com/NixOS/nixpkgs/actions/runs/16223200416/job/45808887124
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make it consistent for now I also disabled it for haskell-updates and pushed the update here.
I thought that the bypass doesn’t work with |
If there are known issues for staging-work with CI, it'd be great if issues in nixpkgs could be created for them and the NixOS/nixpkgs-ci team pinged. We can only take care of what we know about. |
No, it works in our favor, the other way around. Those I don't think we will be able to disable the bypassing for staging branches ever, due to how we do the periodic merges. This would only be possible if we switched to a PR-based periodic merge approach - which I don't see. So once we want to force the status checks without bypassing, we will only do so for master/release. |
Historically reporting those issues hasn’t resulted in any action. But I am grateful for the fact that that has improved recently, of course. I am not sure if there are currently outstanding GHA CI issues for our workflow other than this one, I have just gotten the impression that understanding of the workflow is generally not very widespread and that CI changes have often been made that apply to those branches without full consideration/understanding of it. |
Let me be clear: I am also very open to new suggestions on how to improve the work on staging branches, make it easier, more efficient - you name it. If we can build better tooling for an area as critical, but at the same time as understaffed as staging... I'm more than happy to help. |
|
I appreciate that :) I have some thoughts on this that I’ll have to try and find the time to write up in more detail, but I’ve opened NixOS/nixpkgs#424345 as a fairly trivial thing I think would be a nice improvement. |
3c465a4 to
cb90fec
Compare
|
As a quick-fix I'm leaving it disabled on staging/haskell-updates for now, will come back to this though, an exception for nixpkgs-ci sounds good |
cb90fec to
fa9c84c
Compare
|
@wolfgangwalther @emilazy I now instead added an exception for the nixpkgs-ci app and manually triggered a workflow run, confirming that it works: https://github.com/NixOS/nixpkgs/actions/runs/16331765731/job/46135780251 @NixOS/org I'll need another approval to merge this |
See #134 (comment)
Already applied
Ping @wolfgangwalther @K900 @emilazy @mweinelt