Conversation
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
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.
What did you implement:
This change makes
Deserializertype public. The motivation behind this is thatfrom_env()function is too limiting and doesn't allow any control over the deserialization process.For example consider this program:
It will exit with an error code and text
However if we wrap the deserializer (which we can thanks to https://github.com/dtolnay/path-to-error ) we can get much more information about what happened:
Will print:
Now this is much better, because I can check the my_super_arr value.
I made this change specifically to be able to wrap into
serde_path_to_error, because I have more than 100 env variables and some custom types do not mention name of the field that they failed to parse. For example primitive_types::H160 type says "expected string of length 40 but got 37". As you can imagine checking all the env strings for exact length is a very hard task.But I think there are many other cases where wrapping the deserializer is desirable. Therefore, this type should be public
How did you verify your change:
I used my fork in my app to see if it works correctly
What (if anything) would need to be called out in the CHANGELOG for the next release:
I updated the CHANGELOG, but feel free to edit it whatever you please as I enable the "Allow edits by maintainers" checkbox.