[native] Refactor Duration::toString and DataSize::toString to use fmt::format #24241
Conversation
prestodb-ci
requested review from
a team,
czentgr and
psnv03
and removed request for
a team
aditi-pandit
changed the title
| sizeof(buffer), | ||
| "%.2f%s", | ||
| return fmt::format( | ||
| "{:-f}{}", |
There was a problem hiding this comment.
Use .2 in the formating here.
| sizeof(buffer), | ||
| "%f%s", | ||
| return fmt::format( | ||
| "{:-f}{}", |
There was a problem hiding this comment.
Is "-" needed ? Think its the default.
There was a problem hiding this comment.
@czentgr suggested me to use {:-f} citing : https://fmt.dev/10.2.0/syntax.html
There was a problem hiding this comment.
@anandamideShakyan : Agree with the use of {:-f} syntax. As per the examples in https://fmt.dev/10.2.0/syntax.html#format-examples, the leading "-" is not needed. It is the default. So I was suggesting that we can skip that sign.
| sizeof(buffer), | ||
| "%.2f%s", | ||
| return fmt::format( | ||
| "{:-f}{}", |
There was a problem hiding this comment.
Is "-" needed here ? It is the default.
aditi-pandit
changed the title
|
@anandamideShakyan : Please merge your commits and use the same commit message as the PR title. https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards |
anandamideShakyan
force-pushed
the
dev2
branch
2 times, most recently
from
2cb3281 to
0377f99
Compare
|
@anandamideShakyan : Your code has format-check errors https://app.circleci.com/pipelines/github/prestodb/presto/23646/workflows/08edfa15-4ce4-44cb-8cca-003495536fd6/jobs/98777. Please run |
anandamideShakyan
force-pushed
the
dev2
branch
from
b3cd4b4 to
1002475
Compare
Description
Refactored the Duration::toString and DataSize::toString methods to replace the use of snprintf with fmt::format. This change modernizes the codebase, eliminates the need for manual buffer management, and ensures safer and more readable formatting operations.
Motivation and Context
Refactored to align with modern C++ practices, improving safety, readability, and maintainability. fmt::format eliminates manual buffer management and the risks of buffer overflows inherent in snprintf.
Impact
Simplifies the code, reduces error-prone logic, and aligns with modern C++ standards. The output remains consistent with the original implementation, with minimal performance impact and improved maintainability.
Contributor checklist