-
Notifications
You must be signed in to change notification settings - Fork 552
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
u/named_type: use fmt to format named type #16745
u/named_type: use fmt to format named type #16745
Conversation
8c3204d
to
b4f7280
Compare
`named_type` was previously using a standard stream operator when being printed. This required any type wrapped with `named_type` to have `opertor<<` defined instead of relaying on `fmt::formatter` like a lot of types does. Changed the implementation of named type stream output operator to use `fmt` library. Signed-off-by: Michal Maslanka <michal@redpanda.com>
00b7165
to
96f1068
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.
This looks like a good change, but I'm curious why you also changed the output string to no longer include the leading and trailing { }?
I asked about this some time ago in core channel and got the impression that there was a conclusion that the |
No idea really, just didn't see any note about in the commit so I was curious. LGTM, but maybe amend the message to note why you made this change? |
96f1068
to
f23fe17
Compare
Dropped curly braces that all named types were wrapped with when printed to string. The curly braces that the string representation of `named_type` are redundant and were often on purpose dropped by accessing the underlying value while printing the `named_type`. With this chance we are going to reduce the verbosity of log messages and make them easier to read. Signed-off-by: Michal Maslanka <michal@redpanda.com>
f23fe17
to
1b0d1d7
Compare
This looks fine, but I think future versions of libfmt may start to require a @BenPope I think understands this a little bit more than I do |
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45404#018deb3c-5893-4320-9c27-1bda623de5fd ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45404#018deb3c-588a-4f85-a6b3-0d3b09a15f50 |
That change is going to be vast, this has little bearing on it. |
named_type
was previously using a standard stream operator when being printed. This required any type wrapped withnamed_type
to haveopertor<<
defined instead of relaying onfmt::formatter
like a lot of types does. Changed the implementation of named type stream output operator to usefmt
library.Backports Required
Release Notes