-
Notifications
You must be signed in to change notification settings - Fork 224
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
fix(abci): Add serde default
for Event::type
#1416
Conversation
@@ -18,7 +18,7 @@ pub struct Event { | |||
/// | |||
/// Tendermint calls this the `type`, but we use `kind` to avoid confusion | |||
/// with Rust types and follow Rust conventions. | |||
#[serde(rename = "type")] | |||
#[serde(rename = "type", default)] |
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'm not sure this is an optimal solution, because it's recreating the data format by hand rather than deriving it from the source of truth (the proto messages). See #1415 (comment)
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.
Went for the shortest fix to an immediate issue. But anyone else, please feel free to fix it differently.
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.
Let's go with this fix for now since we are already re-creating the data format by hand everywhere in tendermint-rs.
Maybe we can address this whole thing at once and move to ProtoJSON in a follow-up PR or in cometbft-rs.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1416 +/- ##
=======================================
- Coverage 60.1% 59.5% -0.6%
=======================================
Files 271 272 +1
Lines 26221 26795 +574
=======================================
+ Hits 15768 15963 +195
- Misses 10453 10832 +379 ☔ View full report in Codecov by Sentry. |
Event.type
default
for Event::type
Fixes #1415
.changelog/