Conversation
| /// If such an error occurs, no guarantees are made about how many bytes were read from the source, except that it | ||
| /// is less than `dest.len()`. | ||
| fn read_bytes_into_buffer(&mut self, dest: &mut [u8]) -> Result<()>; | ||
| fn read_bytes_into_exact(&mut self, dest: &mut [u8]) -> Result<()>; |
There was a problem hiding this comment.
When Reece was porting my changes in the past, he changed this name from into_exact to into_buffer, saying that into_exact sounds like an uncommon term.
But, this is a common suffix in Rust. Where a non-exact version will tolerate running out of input,
and an exact version which will not. For example, check out 'https://doc.rust-lang.org/std/io/trait.Read.html#method.read_exact'. Which is basically a mirror of this function.
| #[derive(Debug)] | ||
| #[must_use] | ||
| pub struct Reservation(Range<usize>); |
There was a problem hiding this comment.
This struct represents a reservation of a memory span.
You will never have two of these which overlap, so implementing any kind of Eq is wrong.
Seems like this was just added for testing, but there was a better way to write the test anyways.
| @@ -303,10 +303,10 @@ mod tests { | |||
|
|
|||
| // Assert | |||
| assert!(result.is_err()); | |||
There was a problem hiding this comment.
Instead of directly comparing two errors with eq, now we use matches.
This is more natural, and let's us remove the requirement that all of our errors implement Eq.
Since it's weird to be able to do: error_1 == error_2...
| assert!(reserve_result.is_ok()); | ||
| assert!(write_result.is_ok()); | ||
|
|
||
| assert_eq!(reserve_result.unwrap(), Reservation(0..3)); |
There was a problem hiding this comment.
Necessary to remove the Eq implementation from Reservation.
| @@ -2,23 +2,22 @@ | |||
|
|
|||
| // cspell:ignore Lorem, ipsum, dolor, sit, amet, no, explicari, repudiare, vis, an, dicant, legimus, ponderum | |||
|
|
|||
There was a problem hiding this comment.
These use were never actually used in this scope, just in the nested modules.
So, I moved them into the nested modules where they're actually used.
This fixed a linter warning.
There was a problem hiding this comment.
Pull request overview
This PR performs cleanup work on the slice-codec by:
- Renaming methods for better clarity (
read_bytes_into_buffer→read_bytes_into_exact) - Correcting module naming (
fixed_sized→fixed_size) - Reorganizing imports to be module-scoped rather than file-scoped
- Removing unnecessary trait derivations (
PartialEq,Eq) from error and reservation types - Updating tests to use pattern matching instead of equality comparisons where appropriate
Changes:
- Renamed
read_bytes_into_buffertoread_bytes_into_exactacross all implementations and usages - Fixed grammar in module name from
fixed_sizedtofixed_size - Removed
PartialEqandEqtrait derivations fromErrorKind,InvalidDataErrorKind, andReservationtypes - Updated tests to use
matches!macro for error assertions instead ofassert_eq! - Moved imports from file-level to module-level scope in test files
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| slice-codec/tests/encoding_tests.rs | Renamed test module from fixed_sized to fixed_size and reorganized imports to module scope |
| slice-codec/src/slice2/decoding.rs | Updated method call from read_bytes_into_buffer to read_bytes_into_exact |
| slice-codec/src/error.rs | Removed PartialEq and Eq derives from ErrorKind and InvalidDataErrorKind enums |
| slice-codec/src/buffer/vec.rs | Updated test assertion to use .range() method instead of direct struct comparison |
| slice-codec/src/buffer/slice.rs | Renamed method from read_bytes_into_buffer to read_bytes_into_exact and updated tests to use matches! for error assertions |
| slice-codec/src/buffer/mod.rs | Renamed trait method from read_bytes_into_buffer to read_bytes_into_exact and removed PartialEq derive from Reservation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The slice-codec needed some cleanup even before it was moved into the
slicecrepo, and now that's just even more true.This PR undoes some changes that were made beforehand, that I want to keep separate from the soon-to-follow cleanup.