Conversation
…ds in rocketmq-admin-core
|
🔊@WaterWhisperer 🚀Thanks for your contribution🎉! 💡CodeRabbit(AI) will review your code first🔥! Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
WalkthroughThe PR standardizes command structures across the rocketmq-admin-core module by consistently renaming command types to follow a "SubCommand" convention and refactoring enum variant names to use more concise, direct names while simplifying import paths from fully-qualified module references to direct type imports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/broker_commands/send_msg_status_sub_command.rs (1)
91-91:⚠️ Potential issue | 🟡 MinorInconsistent error message: still references the old name.
Line 91 still uses
"SendMsgStatusCommand: Failed to start producer"while lines 99 and 118 were updated to"SendMsgStatusSubCommand". This should be updated for consistency.Proposed fix
- .map_err(|e| RocketMQError::Internal(format!("SendMsgStatusCommand: Failed to start producer: {}", e)))?; + .map_err(|e| RocketMQError::Internal(format!("SendMsgStatusSubCommand: Failed to start producer: {}", e)))?;
🧹 Nitpick comments (4)
rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/auth_commands.rs (1)
56-61: Minor naming inconsistency: CLI name"copyUser"(singular) vs variantCopyUsers(plural).The command name exposed to users is
copyUser(singular) but the enum variant and type use pluralCopyUsers/CopyUsersSubCommand. Other commands likelistUsers(line 113) are consistently plural. Consider aligning the CLI name to"copyUsers"for consistency, or renaming the variant — unless the singular form is intentional to match an upstream Java convention.rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/consumer_commands.rs (1)
92-98: Nit: Inconsistent match arm variable naming.Lines 92–97 bind the inner value as
cmd, but Line 98 usesvalue. Pick one name consistently across all arms.Suggested fix
- ConsumerCommands::UpdateSubGroup(value) => value.execute(rpc_hook).await, + ConsumerCommands::UpdateSubGroup(cmd) => cmd.execute(rpc_hook).await,rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/controller_commands.rs (1)
64-70: Nit: Inconsistent match arm variable naming.Line 66 binds as
cmdwhile Lines 67–69 usevalue.Suggested fix — use `cmd` everywhere
- ControllerCommands::GetControllerConfig(value) => value.execute(rpc_hook).await, - ControllerCommands::GetControllerMetadata(value) => value.execute(rpc_hook).await, - ControllerCommands::UpdateControllerConfig(value) => value.execute(rpc_hook).await, + ControllerCommands::GetControllerConfig(cmd) => cmd.execute(rpc_hook).await, + ControllerCommands::GetControllerMetadata(cmd) => cmd.execute(rpc_hook).await, + ControllerCommands::UpdateControllerConfig(cmd) => cmd.execute(rpc_hook).await,rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/broker_commands.rs (1)
99-111: Nit: Inconsistent match arm variable naming.Line 105 binds as
cmdwhile all other arms usevalue.Suggested fix
- BrokerCommands::GetBrokerConfig(cmd) => cmd.execute(rpc_hook).await, + BrokerCommands::GetBrokerConfig(value) => value.execute(rpc_hook).await,
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6345 +/- ##
==========================================
- Coverage 42.47% 42.44% -0.04%
==========================================
Files 917 917
Lines 128641 128579 -62
==========================================
- Hits 54645 54574 -71
- Misses 73996 74005 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mxsm
left a comment
There was a problem hiding this comment.
@WaterWhisperer LGTM, 感谢你对RocketMQ-Rust项目的持续贡献,今天除夕祝你和你的家人马年身体健康万事如意。马到成功。
@mxsm 谢谢,也祝您除夕快乐! |
Which Issue(s) This PR Fixes(Closes)
Brief Description
How Did You Test This Change?
Summary by CodeRabbit