refactor: Implement TryFrom for ParameterValue #8420
Merged
Conversation
This file contains 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
nagisa
reviewed
Jan 25, 2023
nagisa
reviewed
Jan 25, 2023
nagisa
reviewed
Jan 25, 2023
nagisa
reviewed
Jan 25, 2023
aborg-dev
force-pushed
the
parameter_try_from
branch
from
b35c1ea to
e34dacb
Compare
nagisa
reviewed
Jan 31, 2023
nagisa
approved these changes
Jan 31, 2023
aborg-dev
force-pushed
the
parameter_try_from
branch
from
9d2ff8f to
12bfd14
Compare
This change simplifies the `ParameterTable::get` method by moving the conversion details from `ParameterValue` to concrete types into the `TryFrom` trait implementation.
aborg-dev
force-pushed
the
parameter_try_from
branch
from
12bfd14 to
a1eb69d
Compare
nikurt
pushed a commit
to nikurt/nearcore
that referenced
this pull request
Feb 2, 2023
This PR implements `TryFrom` for `ParameterValue` for a few types that we need while working with `ParameterTable`. This helps to get rid of a lot of duplication in `get_*` methods, but I wasn't able to get rid of `get_number` that is already generic as it was violating Rust foreign trait rules (see commented out code, the exact error is [1]). Any advice there? I also feel that `TryFrom` implementations have a bit more duplication than `as_*` methods that we had before. Particularly in re-defining the error type each time and constructing very similar error messages. Any advice here would be appreciated. [1] ``` error[E0210]: type parameter `T` must be covered by another type when it appears before the first local type (`ParameterValue`) --> core/primitives/src/runtime/parameter_table.rs:57:6 | 57 | impl<T> TryFrom<&ParameterValue> for T | ^ type parameter `T` must be covered by another type when it appears before the first local type (`ParameterValue`) ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR implements
TryFromforParameterValuefor a few types that we need while working withParameterTable.This helps to get rid of a lot of duplication in
get_*methods, but I wasn't able to get rid ofget_numberthat is already generic as it was violating Rust foreign trait rules (see commented out code, the exact error is [1]). Any advice there?I also feel that
TryFromimplementations have a bit more duplication thanas_*methods that we had before. Particularly in re-defining the error type each time and constructing very similar error messages. Any advice here would be appreciated.[1]