Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: use index override when decoding enums in events #382

Merged
merged 2 commits into from
Jan 12, 2022
Merged

fix: use index override when decoding enums in events #382

merged 2 commits into from
Jan 12, 2022

Conversation

sander2
Copy link
Contributor

@sander2 sander2 commented Jan 4, 2022

Without this fix, if an event contains an enum with explicit indices, it fails to decode

@sander2 sander2 mentioned this pull request Jan 4, 2022
Merged
jsdw
jsdw approved these changes Jan 5, 2022
View reviewed changes
Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jsdw jsdw requested a review from a team January 5, 2022 10:15
ascjones
ascjones reviewed Jan 5, 2022
View reviewed changes
Copy link
Contributor

@ascjones ascjones left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there are already no tests for the events decoding, but I think we need some.

Should be trivial to add a test for this after #384 is merged (which adds unit testing for the EventsDecoder).

@ascjones
Copy link
Contributor

ascjones commented Jan 7, 2022

@sander2 now #384 is merged, you should be able to easily make a test for this now.

@sander2 sander2 force-pushed the enum-variant-decoding branch 2 times, most recently from 8276999 to b43743d Compare January 12, 2022 09:17
niklasad1
niklasad1 reviewed Jan 12, 2022
View reviewed changes
src/events.rs Outdated Show resolved Hide resolved
niklasad1
niklasad1 approved these changes Jan 12, 2022
View reviewed changes
Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sander2 @niklasad1
fix: ignore clippy warning in test
2d5f070 
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
@jsdw jsdw merged commit d1e2985 into paritytech:master Jan 12, 2022
0623forbidden pushed a commit to DEIPworld/substrate-subxt that referenced this pull request Feb 15, 2022
@sander2 @niklasad1 @0623forbidden
fix: use index override when decoding enums in events (paritytech#382)
dec67e8 
* fix: use index override when decoding enums in events

* fix: ignore clippy warning in test

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ascjones ascjones ascjones left review comments

@niklasad1 niklasad1 niklasad1 approved these changes

@jsdw jsdw jsdw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sander2 @ascjones @jsdw @niklasad1