Skip to content

Provide an as_slice method on the Slice type#287

Merged
marvin-j97 merged 2 commits intofjall-rs:mainfrom
irevoire:as_slice-method-on-slice
May 3, 2026
Merged

Provide an as_slice method on the Slice type#287
marvin-j97 merged 2 commits intofjall-rs:mainfrom
irevoire:as_slice-method-on-slice

Conversation

@irevoire
Copy link
Copy Markdown
Contributor

@irevoire irevoire commented Apr 18, 2026

Hey!

I made this small helper similar to the one in the stdlib after having to write buf.extend(&slice as &[u8]); a bunch of times.

Summary by CodeRabbit

  • New Features
    • Added a public read-only accessor that returns the full underlying byte data as a borrowed slice, enabling direct, zero-copy reads.
  • Tests
    • Added a unit test confirming the accessor returns the original byte data unchanged.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 79d84a24-8f60-40a9-a310-fa33dde6dad4

📥 Commits

Reviewing files that changed from the base of the PR and between 05c082f and a8db088.

📒 Files selected for processing (2)
  • src/slice/slice_bytes/mod.rs
  • src/slice/slice_default/mod.rs

📝 Walkthrough

Walkthrough

Adds a public as_slice(&self) -> &[u8] method (annotated #[must_use]) to Slice implementations and a unit test that verifies it returns the underlying bytes unchanged.

Changes

Cohort / File(s) Summary
Default slice impl
src/slice/slice_default/mod.rs
Added pub fn as_slice(&self) -> &[u8] on Slice, returning a borrowed &[u8] of the underlying data and marked #[must_use].
Bytes-backed slice impl
src/slice/slice_bytes/mod.rs
Added pub fn as_slice(&self) -> &[u8] on Slice, exposing the full underlying byte slice and annotated #[must_use].
Tests
src/slice/mod.rs
Added a unit test constructing a Slice from an array and asserting as_slice() returns the original bytes unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding an as_slice method to the Slice type, which is confirmed by the changes across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/slice/slice_default/mod.rs 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@marvin-j97
Copy link
Copy Markdown
Contributor

marvin-j97 commented Apr 22, 2026

This should probably be implemented for Slice in general, not just the byteview flavour.

Comment thread src/slice/slice_default/mod.rs
@irevoire
Copy link
Copy Markdown
Contributor Author

Thanks for the review!
I'm not super available right now but will look into it in a few days!

@irevoire irevoire force-pushed the as_slice-method-on-slice branch from 43000ad to 05c082f Compare April 27, 2026 14:37
@irevoire irevoire requested a review from marvin-j97 April 27, 2026 14:38
@marvin-j97 marvin-j97 merged commit ad29f80 into fjall-rs:main May 3, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants