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.
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
feat(pkg/scale): encoding and decoding of maps in scale #2894
feat(pkg/scale): encoding and decoding of maps in scale #2894
Changes from 5 commits
a7fc7b0
27ead7c
868ca44
12009fe
4aae1e5
00c8133
10c98f4
e2e0117
da9e32e
ec37a64
d69371d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Shouldn't we return an error in that case, to avoid silently discarding a map value when encoding?
@timwu20
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.
Def wait for Tims answer, but this happens at other places in this file as well (i.e. structs)
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.
CanInterface
is essentially determining if this is a public attribute of a struct or public method. I think it should be fine in this case. This will almost always return true in the case we're trying to decode into a map value. I wonder what cases this would return false though, maybe we can provide a test case.@kishansagathiya please don't merge the PR with unresolved conversations. My bad for not getting to this earlier.
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.
I realised about this comment few moments after merging it. Wasn't deliberate.