Move value-only methods to InstallableValue#7757
Merged
edolstra merged 1 commit intoNixOS:masterfrom Mar 24, 2023
Merged
Conversation
7 tasks
|
🎉 All dependencies have been resolved ! |
Ericson2314
force-pushed
the
more-installable-value
branch
from
e7cd4e9 to
403bf9d
Compare
tomberek
reviewed
Feb 24, 2023
| virtual std::pair<Value *, PosIdx> toValue(EvalState & state) | ||
| { | ||
| throw Error("argument '%s' cannot be evaluated", what()); | ||
| } |
Contributor
There was a problem hiding this comment.
notes: this is the main motivation, remove default implementation of an error
Ericson2314
force-pushed
the
more-installable-value
branch
2 times, most recently
from
1ea7208 to
a0f6067
Compare
|
🎉 All dependencies have been resolved ! |
Ericson2314
force-pushed
the
more-installable-value
branch
from
a0f6067 to
1c6a631
Compare
These methods would previously fail on the other `Installable`s, so moving them to this class is more correct as to where they actually work. Additionally, a `InstallableValueCommand` is created to make it easier (or rather no worse than before) to write commands that just work on `InstallableValue`s. Besides being a cleanup to avoid failing default methods, this gets us closer to NixOS/rfcs#134.
Ericson2314
force-pushed
the
more-installable-value
branch
from
1c6a631 to
c998e01
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
These methods would previously fail on the other
Installables, so moving them to this class is more correct as to where they actually work.Additionally, a
InstallableValueCommandis created to make it easier (or rather no worse than before) to write commands that just work onInstallableValues.Context
Besides being a cleanup to avoid failing default methods, this gets us closer to NixOS/rfcs#134.
Depends on #7750Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.shsrc/*/teststests/nixos/*