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 a TypeNameToDebug formatter to zebra_chain #2466

Merged
merged 2 commits into from
Jul 9, 2021
Merged

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jul 8, 2021

Motivation

This formatter makes it much easier to diagnose proptest errors. Without this formatter, the output of proptest failures can extend across hundreds of terminal lines.

This formatter will be used in a future PR to proptest the double-spend checks.

Change Risk

This is a low-risk change, because these formatters are only used in the tests and logs.

Solution

  • Add a TypeNameToDebug formatter to zebra_chain
  • Implement Arbitrary and DerefMut for all the formatters
  • Make the formatter type bounds consistent, to produce better compiler errors

This is part of ticket #2231, but it does not close that ticket.

Review

Anyone can review this PR. It's not urgent.

Reviewer Checklist

  • Formatters work as documented

These methods will be tested as part of the double-spend tests in a future PR.

Follow Up Work

Actually check for double-spends

This formatter makes it much easier to diagnose proptest errors.
It will be used in a future PR.

Implement Arbitrary and DerefMut for all the formatters.

Also make the formatter type bounds consistent,
to produce better compiler errors.
@teor2345 teor2345 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium labels Jul 8, 2021
@teor2345 teor2345 requested a review from jvff July 8, 2021 05:38
@teor2345 teor2345 self-assigned this Jul 8, 2021
jvff
jvff previously approved these changes Jul 9, 2021
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Looks good. I only added a small optional nit-pick in the phrasing of the documentation to make it a bit clearer.

zebra-chain/src/fmt.rs Outdated Show resolved Hide resolved
Co-authored-by: Janito Vaqueiro Ferreira Filho <[email protected]>
@teor2345 teor2345 requested a review from jvff July 9, 2021 02:14
@teor2345 teor2345 merged commit 64be7fd into main Jul 9, 2021
@teor2345 teor2345 deleted the type-name-to-debug branch July 9, 2021 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants