[ISSUE #3529]♻️Refactor HAConnection start method to use match statement for cleaner connection handling#3530
Conversation
…ent for cleaner connection handling
rocketmq-rust-bot
requested review from
SpaceXCN,
TeslaRustor and
rocketmq-rust-bot
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the HAConnection start method to improve clarity by using a match statement for cleaner connection handling.
- Replaces the if/else chain with a match statement to select the appropriate active connection.
- Maintains the original functionality while presenting the connection selection logic in a more maintainable and explicit way.
|
🔊@mxsm 🚀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💥. |
rocketmq-rust-bot
added
AI review first
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GeneralHAConnection
participant DefaultHAConnection
participant AutoSwitchHAConnection
Caller->>GeneralHAConnection: start()
alt default_ha_connection is Some
GeneralHAConnection->>DefaultHAConnection: start()
else auto_switch_ha_connection is Some
GeneralHAConnection->>AutoSwitchHAConnection: start()
else both are None
GeneralHAConnection-->>Caller: return error
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rocketmq-store/src/ha/general_ha_connection.rs (1)
66-75: Consider collapsing the tuple-match into a singleOptionchainThe current
matchis perfectly valid and improves readability compared to the old nestedif-lets.
That said, you still need two mutable borrows at pattern-matching time, and you hard-code the priority (default over auto-switch) twice.A slightly leaner alternative avoids the tuple borrow and encodes the same priority in one expression:
- match ( - &mut self.default_ha_connection, - &mut self.auto_switch_ha_connection, - ) { - (Some(connection), _) => connection.start().await, - (_, Some(connection)) => connection.start().await, - (None, None) => Err(HAConnectionError::Connection( - "No HA connection set".to_string(), - )), - } + self.default_ha_connection + .as_mut() + .or(self.auto_switch_ha_connection.as_mut()) + .map(|conn| conn.start()) + .unwrap_or_else(|| async { + Err(HAConnectionError::Connection("No HA connection set".into())) + }) + .awaitBenefits:
- Borrows only one mutable reference at a time.
- Encapsulates the priority order in a single
orcall.- Reduces the LOC while keeping intent explicit.
Purely optional, but worth considering for terseness and borrow simplicity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rocketmq-store/src/ha/general_ha_connection.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build
- GitHub Check: test
- GitHub Check: build (macos-latest, stable)
- GitHub Check: build (windows-latest, nightly)
- GitHub Check: build (windows-latest, stable)
- GitHub Check: build (macos-latest, nightly)
- GitHub Check: build (ubuntu-latest, stable)
- GitHub Check: build (ubuntu-latest, nightly)
- GitHub Check: auto-approve
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3530 +/- ##
=======================================
Coverage 26.18% 26.18%
=======================================
Files 556 556
Lines 78651 78651
=======================================
Hits 20593 20593
Misses 58058 58058 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rocketmq-rust-bot
added
approved
Which Issue(s) This PR Fixes(Closes)
Fixes #3529
Brief Description
How Did You Test This Change?
Summary by CodeRabbit