-
Notifications
You must be signed in to change notification settings - Fork 143
[ISSUE #1409]🔥Remove unused crate in rocketmq-namesrv🚨 #1410
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
Conversation
WalkthroughThe Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥 |
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
rocketmq-namesrv/Cargo.toml (1)
27-27
: Fix typo in commentThe comment has a typo: "spupport" should be "support"
-#json spupport +#json support
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
rocketmq-namesrv/Cargo.toml
(1 hunks)
🔇 Additional comments (1)
rocketmq-namesrv/Cargo.toml (1)
Line range hint 1-42
: Verify removal of dependencies
The removal of several dependencies needs verification to ensure they're not used in the codebase. Additionally, since env_logger
was removed, we should verify that logging functionality is handled by another dependency.
If any of these dependencies are still in use, they need to be retained or their usages need to be refactored. If no matches are found, the removal is safe.
Consider documenting the logging setup in the README.md since env_logger was removed. This will help other contributors understand how logging is configured in the project.
✅ Verification successful
Let me gather more information about the logging setup to ensure it's properly handled after removing env_logger.
Let me check the common logging implementation since I see it's using rocketmq_common::log::init_logger()
.
Let me check the workspace Cargo.toml to see if env_logger is still referenced there.
Dependencies safely removed and logging properly handled
The verification shows that:
- None of the removed dependencies (tokio_util, tokio_stream, futures, etc.) are used in the namesrv codebase
- Logging is properly handled through
rocketmq_common::log::init_logger()
which uses tracing-subscriber - While env_logger is listed in the workspace Cargo.toml, it's not actually used in the namesrv code
The dependency removals are safe, and logging functionality is properly maintained through the common logging implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that removed dependencies are not used in the codebase
# and check logging setup
# Check for any remaining usage of removed dependencies
echo "Checking for usage of removed dependencies..."
rg -l 'tokio_util|tokio_stream|futures::|futures_core|futures_sink|futures_io|futures_util|env_logger' ./rocketmq-namesrv/src
# Check for logging setup to ensure it's handled after env_logger removal
echo "Checking current logging setup..."
ast-grep --pattern 'tracing::subscriber::set_global_default($$$)' ./rocketmq-namesrv/src
Length of output: 396
Script:
#!/bin/bash
# Check for any logging initialization or setup in the codebase
echo "Checking for tracing initialization..."
rg -l "tracing::(init|setup|debug|info|warn|error)" ./rocketmq-namesrv/src
echo "Checking for other logging setups..."
rg -l "log::|logger::" ./rocketmq-namesrv/src
# Check the bootstrap file specifically
echo "Checking namesrv_bootstrap_server.rs content..."
cat ./rocketmq-namesrv/src/bin/namesrv_bootstrap_server.rs
Length of output: 3550
Script:
#!/bin/bash
# Check the common logging implementation
echo "Checking common logging implementation..."
rg -A 10 "init_logger" ./rocketmq-common/src/
# Check if there are any remaining env_logger references in the entire project
echo "Double checking env_logger usage..."
rg -l "env_logger" .
Length of output: 894
Script:
#!/bin/bash
# Check the workspace Cargo.toml for env_logger
echo "Checking workspace Cargo.toml..."
cat ./Cargo.toml
# Also check if there are any actual uses of env_logger in the code
echo "Checking for actual env_logger usage in code..."
rg -C 3 "env_logger" .
Length of output: 3037
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1410 +/- ##
=======================================
Coverage 21.11% 21.11%
=======================================
Files 443 443
Lines 57146 57146
=======================================
Hits 12069 12069
Misses 45077 45077 ☔ View full report in Codecov by Sentry. |
Which Issue(s) This PR Fixes(Closes)
Fixes #1409
Brief Description
How Did You Test This Change?
Summary by CodeRabbit
anyhow
,tokio
,tracing
,serde
, andserde_json
.