Remove all explicit usage of fmtlib #1724
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harrism
added
non-breaking
wence-
requested changes
Nov 11, 2024
include/rmm/mr/device/detail/stream_ordered_memory_resource.hpp
Outdated
Show resolved
Hide resolved
wence-
reviewed
Nov 13, 2024
3 tasks
jameslamb
reviewed
Nov 13, 2024
3 tasks
|
It looks like you found a number of the issues with my snprintf implementation that I have since fixed 😂 Hopefully I caught everything, I'll review now and see. |
vyasr
requested changes
Nov 13, 2024
There was a problem hiding this comment.
Overall looks great. I have a few small suggestions, and I'm blocking merge per discussion of #1722 (comment), but can approve once we resolve either way.
@vyasr none of your comments read as suggestions, just comments on differences from your PR. Are there any specific changes you are requesting? |
|
I resolved the indeterminate threads and left open the two that require nonzero resolution. |
|
/merge |
rapids-bot bot
pushed a commit
that referenced
this pull request
Nov 14, 2024
…replay benchmark (#1728) Fixes #1727 Contributes to rapidsai/build-planning#26 - Removes `-Wno-error=deprecated-declarations` - Replaces deprecated usage of `rmm::logger()` in reply benchmark with supported `RMM_LOG_INFO` macros. Note the latter duplicates a change in #1724 which allows the two PRs to be merged independently. Authors: - Mark Harris (https://github.com/harrism) Approvers: - Rong Ou (https://github.com/rongou) - Bradley Dice (https://github.com/bdice) URL: #1728
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1717
Also fixes #1710 in 5330063
I have replaced fmt-style format string placeholders (
"... {} ...") with printf-style placeholders by adding a functionrmm::detail::formatted_log(), which I modified from @vyasr 's #1722.The only remaining mention of fmt is in CMakeLists.txt. Do we still need to explicitly fetch fmt?Removed.
Checklist