-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat(encoding): impl EncodeLabelValue
for bool
#237
feat(encoding): impl EncodeLabelValue
for bool
#237
Conversation
0835246
to
b90a1d8
Compare
impl EncodeLabelValue for bool { | ||
fn encode(&self, encoder: &mut LabelValueEncoder) -> Result<(), std::fmt::Error> { | ||
encoder.write_str(if *self { "true" } else { "false" }) | ||
} | ||
} |
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.
for the record, i would support this addition, i think it's a pleasant and natural addition to the library.
that said, the reordering of impl
blocks here is probably best left for a distinct pull request, as it introduces a lot of undue noise for reviewers to inspect.
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.
Thank you.
Can you please add a changelog entry to the commit adding the impl for bool
.
Please keep the split into two commits.
b90a1d8
to
6898722
Compare
There are some clippy warnings introduced by rust 1.83 during CI check. |
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!
One nit pick. Otherwise ready to merge.
Mind updating to latest master
, or giving me access to merge master
into this pull request?
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
6898722
to
96796bc
Compare
EncodeLabelValue
forbool