[ISSUE #3408]Refactor QueryMessageRequestHeader with derive marco RequestHeaderCodec #3476
Conversation
… RequestHeaderCodec
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QueryMessageRequestHeader
participant RequestHeaderCodec (macro)
User->>QueryMessageRequestHeader: Create instance with required fields
QueryMessageRequestHeader->>RequestHeaderCodec (macro): Serialize/Deserialize via macro
RequestHeaderCodec (macro)-->>QueryMessageRequestHeader: Handles encoding/decoding
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
✨ 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
|
|
🔊@donghao526 🚀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-robot
added
Difficulty level/Easy
rocketmq-rust-bot
added
auto merge
ready to review
waiting-review
![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 (3)
rocketmq-remoting/src/protocol/header/query_message_request_header.rs (3)
24-44: Clarify required‐field semanticsAll business fields are now marked
#[required].
Double-check that the derive macro enforces this at both encode and decode time; otherwise deserialisation may silently accept missing data and lead to runtimeunwrap()s downstream. If the macro already validates, please add a brief doc-comment on the struct noting that these fields are mandatory so that callers understand the contract.
46-52: Remove large commented blocksThe old constant definitions and hand-written
CommandCustomHeader/FromMapimplementations are no longer useful and bloat the file. Deleting them instead of commenting keeps history in git while leaving the codebase clean.Also applies to: 60-149
151-242: Formatting & minor test cleanupCI reports
cargo fmtfailure on this file.
Runcargo fmt --allbefore merging.In the tests, the explicit type annotations (
let header: QueryMessageRequestHeader = …) are unnecessary; the compiler can infer them. Removing them makes the tests slightly cleaner.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rocketmq-remoting/src/protocol/header/query_message_request_header.rs(5 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
rocketmq-remoting/src/protocol/header/query_message_request_header.rs
[error] 151-195: cargo fmt check failed due to formatting differences. Please run 'cargo fmt' to format the code.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: test
- GitHub Check: auto-approve
🔇 Additional comments (1)
rocketmq-remoting/src/protocol/header/query_message_request_header.rs (1)
18-19: Macro import looks good
rocketmq_macros::RequestHeaderCodecis correctly brought into scope.
No action needed here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3476 +/- ##
==========================================
- Coverage 26.66% 26.60% -0.06%
==========================================
Files 546 546
Lines 78050 77978 -72
==========================================
- Hits 20813 20749 -64
+ Misses 57237 57229 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🔊@donghao526 🚀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
approved
Which Issue(s) This PR Fixes(Closes)
Fixes #3408
Brief Description
Refactor QueryMessageRequestHeader with derive marco RequestHeaderCodec
How Did You Test This Change?
Running the unit tests local
Summary by CodeRabbit