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.

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

Add matching serializer for app_hash field of block::Header struct #188

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

romac
Copy link
Member

@romac romac commented Mar 19, 2020

Closes: #187

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs (N/A)
  • Updated all code comments where relevant (N/A)
  • Wrote tests
  • Updated CHANGES.md (N/A)

@romac romac requested a review from liamsi March 19, 2020 15:20
fn serialization_roundtrip() {
let json_data = include_str!("../../tests/support/serialization/block/header.json");
test_serialization_roundtrip::<Header>(json_data);
}
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

Very small change with relatively a lot of test code. All our PRs should be a bit more like @romac's :-) 👍

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@50cd585). Click here to learn what that means.
The diff coverage is 56.25%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #188   +/-   ##
=========================================
  Coverage          ?   39.72%           
=========================================
  Files             ?       98           
  Lines             ?     3522           
  Branches          ?      559           
=========================================
  Hits              ?     1399           
  Misses            ?     1722           
  Partials          ?      401
Impacted Files Coverage Δ
tendermint/src/block/header.rs 20.83% <100%> (ø)
tendermint/src/test.rs 41.66% <41.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50cd585...54186f2. Read the comment docs.

@liamsi liamsi merged commit 7df19f8 into master Mar 19, 2020
@liamsi liamsi deleted the romac/fix-app-hash-serialization branch March 19, 2020 15:52
@liamsi liamsi mentioned this pull request Mar 29, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Serialize and Deserialize implementations of block::header::Header do not match
3 participants