Conversation
Replace Cursor and ReadBytesExt with direct byte slice parsing using from_be_bytes/from_le_bytes. Removes byteorder dependency and updates docs.
Reviewer's GuideThis PR streamlines length‐prefix parsing by removing the byteorder dependency, refactors bytes_to_u64 to use direct slice conversions via parse_bytes and native from_be_bytes/from_le_bytes, updates documentation examples to match the new approach, and cleans up formatting across markdown files. Class diagram for removal of byteorder dependencyclassDiagram
class byteorder {
-BigEndian
-LittleEndian
-ReadBytesExt
}
class frame {
-byteorder dependency removed
}
frame --|> byteorder : removed
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes remove the dependency on the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FrameModule
Caller->>FrameModule: bytes_to_u64(slice, size, endian)
alt Supported size and valid slice
FrameModule->>FrameModule: Convert slice to array
FrameModule->>FrameModule: Use from_be_bytes/from_le_bytes
FrameModule-->>Caller: Return u64 value
else Unsupported size or short slice
FrameModule-->>Caller: Return error
end
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)`**/*.rs`: Comment why, not what. Explain assumptions, edge cases, trade-offs, o...
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
`**/*.rs`: * Seek to keep the cyclomatic complexity of functions no more than 12...
⚙️ Source: CodeRabbit Configuration File List of files the instruction was applied to:
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 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
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/frame.rs:55` </location>
<code_context>
/// let buf = [0x00, 0x10, 0x20, 0x30];
/// assert_eq!(bytes_to_u64(&buf, 2, Endianness::Big).unwrap(), 0x0010);
/// ```
+///
+/// # Panics
+/// Panics if a valid `size` is provided but `bytes` is shorter than `size`.
+/// This should never occur as the length check above guards against it.
pub fn bytes_to_u64(bytes: &[u8], size: usize, endianness: Endianness) -> io::Result<u64> {
if !matches!(size, 1 | 2 | 4 | 8) {
</code_context>
<issue_to_address>
Documented panic is technically unreachable but could be enforced with debug_assert.
Adding debug_assert!(bytes.len() >= size) before the match would make this invariant explicit and help catch logic errors during development.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Since you already return an Err when
bytes.len() < size, thedebug_assert!for slice length is redundant—consider removing it to simplify the code. - A small helper or macro for converting the first N bytes into a u64 (e.g.
fn parse_bytes<const N: usize>(bytes: &[u8], f: fn([u8; N]) -> u64)) could reduce the repetition across the match arms.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since you already return an Err when `bytes.len() < size`, the `debug_assert!` for slice length is redundant—consider removing it to simplify the code.
- A small helper or macro for converting the first N bytes into a u64 (e.g. `fn parse_bytes<const N: usize>(bytes: &[u8], f: fn([u8; N]) -> u64)`) could reduce the repetition across the match arms.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Remove debug assertion and introduce helper for converting prefix bytes. Improves clarity and reduces repetition.
|
@sourcery-ai review |
1 participant
Summary
Testing
make fmtmake lintmake testnixie *.md **/*.mdhttps://chatgpt.com/codex/tasks/task_e_686a872b0e648322b158f6130fc906d1
Summary by Sourcery
Remove the byteorder dependency and replace cursor-based parsing in bytes_to_u64 with direct slice-to-integer conversions, and update documentation examples and formatting accordingly.
Enhancements:
Build:
Documentation: