-
Notifications
You must be signed in to change notification settings - Fork 412
Implement core::fmt::Display for HumanReadableName #3829
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
Implement core::fmt::Display for HumanReadableName #3829
Conversation
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
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.
Basically LGTM, one comment.
@@ -287,6 +287,12 @@ impl Readable for HumanReadableName { | |||
} | |||
} | |||
|
|||
impl std::fmt::Display for HumanReadableName { |
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.
As we want to keep this crate no_std
-compatible, please use core::fmt
instead of std::fmt
. Any uses of std
would need to be feature-gated behind the std
feature, but in this case there is no reason to.
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.
Okay will do, thanks!
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
209f0f2
to
004937b
Compare
Feel free to squash the fixups, which should fix CI. |
004937b
to
3484eb0
Compare
I think I need to add a test case that asserts the correct HRN display format, for mutation testing to pass in CI. |
3484eb0
to
81b201a
Compare
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.
Thanks!
Closes #3823
This PR implements
Display
forHumanReadableName
which allows us to print a HRN as₿user@domain
.