-
Notifications
You must be signed in to change notification settings - Fork 185
Zlib-rs round 2 #522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: copilot/add-tests-for-zlib-rs-functionality
Are you sure you want to change the base?
Zlib-rs round 2 #522
Conversation
|
So, should |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive support for zlib-rs features, including dictionary support and dynamic compression level adjustment. The changes integrate unreleased zlib-rs functionality and improve feature flag documentation.
Key Changes
- Added
set_dictionary()methods for both compression and decompression with zlib-rs backend - Added
set_level()method support for zlib-rs backend with proper feature gating - Enhanced feature flag documentation in Cargo.toml with comprehensive descriptions of each backend option
- Improved total_in/total_out byte tracking to exclude dictionary bytes in zlib-rs backend
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/mem.rs | Added zlib-rs-specific set_dictionary and refactored set_level with feature-gated implementations for both zlib-rs and C backends |
| src/lib.rs | Updated backend documentation to mention feature flags and added feature selection priority order documentation |
| src/ffi/zlib_rs.rs | Implemented custom total_in/total_out tracking to exclude dictionary bytes, added dictionary and level setting methods with proper error handling |
| Cargo.toml | Switched to unreleased zlib-rs git dependency, added document-features dependency, and comprehensively documented all feature flags with usage guidance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks so much @folkertdev for getting this going! I can't really tell if it's good like this, but can say 'it looks good'. Thus, I think we'd want some more validation. My idea here would be to adjust the I hope that makes sense, if not, let's make adjustments until it does :). |
d8a67e7 to
816bd26
Compare
a637889 to
fc06575
Compare
fc06575 to
92af1df
Compare
|
I've made the zlib-rs release now, so from that perspective this is ready. The At this point, what does that separate zlib-rs test file add? |
Uses unreleased zlib-rs code, so we'll have to wait with that.
But before I make a release: does this look right? Are there any other missing things?