-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[FMT] Fix warnings about deprecated use of fmt::format_to()
#35377
Conversation
fmt::format_to()
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35377/25466
|
A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master. It involves the following packages:
@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-58ed34/18854/summary.html Comparison SummarySummary:
|
If the deprecated function is used only in tests, I think this is fine. Even if used in production I'd argue we have other problems if 5 % increase in string formatting would be a problem :) |
+core |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
This should fix the compiler warnings about the deprecated use of
fmt::format_to
[a] .@makortel , although this fixes the warning but according to fmtlib/fmt#2420 it causes 5% performance losses. So better to check if this deprecated
fmt
api should be used or not. Currently it is only used in one unit test.[a]