chore: cleanup results#77
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR is a small cleanup pass that standardizes result types and minor naming/import usage across the Rust and C++ codec layers, plus a couple of documentation/comment tidy-ups.
Changes:
- Standardize public/internal APIs to return
FastPForResult<T>instead of spelling outResult<_, FastPForError>. - Minor refactors/cleanup: import
bytemuck::cast_slice(_mut)directly, and renamethissize→this_sizein FastPFOR internals. - Remove outdated crate-level TODO comment and reorder the README supported-algorithms list.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/decode_error_paths.rs | Imports cast_slice_mut and uses it directly to mutate compressed buffers in tests. |
| src/rust/integer_compression/fastpfor.rs | Uses cast_slice directly and renames thissize to this_size for clarity. |
| src/lib.rs | Cleans up feature-gate cfg syntax and removes an outdated FIXME block comment. |
| src/helpers.rs | Switches helper traits/methods to use FastPForResult alias consistently. |
| src/cpp/wrappers.rs | Updates C++ wrapper helpers to return FastPForResult. |
| src/cpp/codecs.rs | Updates codec macro impls to return FastPForResult; tweaks a test comment. |
| src/codec.rs | Updates trait signatures and docs to use FastPForResult. |
| README.md | Reorders supported algorithm bullet list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /// C++ `fastpfor256_codec` returns `CompositeCodec<FastPFor<8>, VariableByte>` — already | ||
| /// any-length. Use it directly; do not wrap in Rust `CppComposite`. | ||
| /// any-length. Use it directly; do not wrap in Rust `CompositeCodec`. |
There was a problem hiding this comment.
This test comment mentions wrapping in Rust CompositeCodec, but there is no CompositeCodec type in this crate (searching the source only finds references in docs/comments). Consider rewording to avoid naming a non-existent Rust wrapper type (e.g., “do not wrap it again on the Rust side; it already supports any-length input”).
| /// any-length. Use it directly; do not wrap in Rust `CompositeCodec`. | |
| /// any-length. Use it directly; do not wrap it again on the Rust side; it already supports any-length input. |
No description provided.