Skip to content
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

Switch to using separate rapids-logger repo #1774

Merged
merged 4 commits into from
Jan 2, 2025

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Dec 30, 2024

Description

This PR removes rmm's logger code in favor of using the rapids-logger repo to which that code was moved. The main material change is that with the latest commit on that repo rmm will dump output to stderr instead of to a file by default, which was the generally requested behavior and also aligns with the rest of RAPIDS's loggers pre-rapids-logger. Nonetheless, I've marked that as a breaking change (also because the rapids-logger code is no longer available from this repository).

Contributes to rapidsai/build-planning#104.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vyasr vyasr added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Dec 30, 2024
@vyasr vyasr self-assigned this Dec 30, 2024
@vyasr vyasr requested a review from a team as a code owner December 30, 2024 23:11
@github-actions github-actions bot added the CMake label Dec 30, 2024
@vyasr vyasr added breaking Breaking change and removed non-breaking Non-breaking change labels Dec 31, 2024
CMakeLists.txt Show resolved Hide resolved
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to unblock further work. I did not read the deleted code, just the CMake changes to use an external repo instead of the vendored code.

@bdice
Copy link
Contributor

bdice commented Jan 1, 2025

@vyasr Can you update the RMM README, too?

rmm/README.md

Line 650 in ba35f8e

etc. The default log file is `rmm_log.txt` in the current working directory, but the environment

@vyasr
Copy link
Contributor Author

vyasr commented Jan 2, 2025

@vyasr Can you update the RMM README, too?

rmm/README.md

Line 650 in ba35f8e

etc. The default log file is `rmm_log.txt` in the current working directory, but the environment

Good call, yes.

@vyasr
Copy link
Contributor Author

vyasr commented Jan 2, 2025

/merge

@rapids-bot rapids-bot bot merged commit 8275ba8 into rapidsai:branch-25.02 Jan 2, 2025
61 checks passed
@vyasr vyasr deleted the feat/use_rapids_logger branch January 2, 2025 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change CMake improvement Improvement / enhancement to an existing function
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants