meta.problems: Custom package problems [RFC127]#478539
meta.problems: Custom package problems [RFC127]#478539infinisil merged 6 commits intoNixOS:masterfrom
Conversation
| - "deprecated": The package relies on software which has reached its end of life. | ||
| - "maintainerless": Automatically generated for packages with `meta.maintainers == []`. Unique, not manually specifiable. | ||
|
|
||
| Each problem has a handler that deals with it, which can be one of "error", "warn" or "ignore". |
There was a problem hiding this comment.
Nit: I think I'd prefer if this was called "severity".
Declaring what it is rather than what should be done with it feels more right to me.
There was a problem hiding this comment.
"Handler" also sort of implies that what's declared is an action or a command; perhaps even a custom one.
"Severity" OTOH is just a property of the problem itself.
There was a problem hiding this comment.
I like the idea. It's a derivation from the RFC, but I don't think we need to follow it to the dot.
There was a problem hiding this comment.
Although, this also implies that we should change config.problems.handlers, and I'm not sure I want to go that far.
|
Ping, would be great to get this merged swiftly so that people can try it out |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
ConnorBaker
left a comment
There was a problem hiding this comment.
Mostly nits/questions
There was a problem hiding this comment.
I'm pretty happy with this error message now tbh, but we can of course adjust this later if wanted
a036cd4 to
e382faf
Compare
|
I've addressed all feedback now and would like to merge this soon. I'll also try to get follow-ups to this done, including migration of |
The attributes of the warning/invalid values are never used anywhere, and maintaining these would make future changes harder
Not just used for errors anymore
See https://github.com/NixOS/rfcs/blob/master/rfcs/0127-issues-warnings.md Co-Authored-By: piegames <git@piegames.de> Co-Authored-By: AkechiShiro <14914796+AkechiShiro@users.noreply.github.com>
|
Pushed to fix conflict. I'll go ahead with the merge now, I hope nothing breaks from this! |
|
There's some spurious GitHub Actions failures, let's try again.. |
|
Follow-up for |
This was an oversight in NixOS#478539 that became apparent in NixOS#494416: If there's a failing problem in Nixpkgs packages, CI will call handleEvalIssue on it, but the problem error was missing a `.reason`. Though even if it did have a reason, we need to do more to make sure we don't break any code that uses it, so the new code uses the problem kind as the reason, which happens to match with the reason for all expected problem kinds.
This was an oversight in NixOS#478539 that became apparent in NixOS#494416: If there's a failing problem in Nixpkgs packages, CI will call handleEvalIssue on it, but the problem error was missing a `.reason`. Though even if it did have a reason, we need to do more to make sure we don't break any code that uses it, so the new code uses the problem kind as the reason, which happens to match with the reason for all expected problem kinds.
This PR implements RFC127, originally based on @piegamesde's initial implementation in #177272 and @AkechiShiro's follow up to add some tests in #338267.
This PR improves over the above by:
Things done
nix-build -A tests.problemsAdd a 👍 reaction to pull requests you find important.