Specify clippy MSRV #17
Conversation
Signed-off-by: NxPKG <[email protected]>
Reviewer's Guide by SourceryThis pull request specifies the Minimum Supported Rust Version (MSRV) for Clippy by adding a No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@NxPKG has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request introduces a new configuration entry in the Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @NxPKG - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment explaining why this MSRV was chosen.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
CI Feedback 🧐(Feedback updated until commit 050df46)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Signed-off-by: NxPKG <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
bongonet-header-serde/src/lib.rs (5)
99-104: Well-designed enum for compression strategies.The enum provides a good abstraction for the two compression scenarios. Consider adding documentation comments to explain the purpose and usage of each variant.
// Wrapper type to unify compressing with and withuot a dictionary, // since the two structs have different inputs for their APIs. +/// Enum representing different compression strategies. +/// +/// This enum encapsulates two different approaches to compression: +/// - Default: Uses standard compression without a dictionary +/// - WithDict: Uses compression with a trained dictionary for better compression ratios enum ZstdCompression { + /// Standard compression with specified compression level Default(thread_zstd::Compression, i32), + /// Compression with a trained dictionary for improved compression WithDict(thread_zstd::CompressionWithDict), }
114-117: Fix error context message in compression.The error context message in the
compressmethod incorrectly states "decompress header" which doesn't match the operation being performed.fn compress(&self, data: &[u8]) -> Result<Vec<u8>> { match &self { ZstdCompression::Default(c, level) => c .compress(data, *level) - .map_err(|e| into_error(e, "decompress header")), + .map_err(|e| into_error(e, "compress header")), ZstdCompression::WithDict(c) => c
118-120: Fix error context message in compression (WithDict variant).Similar to the previous comment, the error context message for the
WithDictvariant also needs correction.ZstdCompression::WithDict(c) => c .compress(data) - .map_err(|e| into_error(e, "decompress header")), + .map_err(|e| into_error(e, "compress header")),
111-133: Consider reducing code duplication in error handling.Both methods of the
ZstdCompressionenum have similar error handling patterns. You could potentially refactor this to reduce duplication, especially if more variants are added in the future.Here's a possible approach using a helper method:
impl ZstdCompression { + // Helper method to standardize error handling + fn handle_error<T>(&self, result: std::result::Result<T, &'static str>, context: &'static str) -> Result<T> { + result.map_err(|e| into_error(e, context)) + } fn compress(&self, data: &[u8]) -> Result<Vec<u8>> { match &self { ZstdCompression::Default(c, level) => c .compress(data, *level) - .map_err(|e| into_error(e, "compress header")), + .map_err(|e| into_error(e, "compress header")), ZstdCompression::WithDict(c) => c .compress(data) - .map_err(|e| into_error(e, "compress header")), + .map_err(|e| into_error(e, "compress header")), } } fn decompress_to_buffer(&self, source: &[u8], destination: &mut Vec<u8>) -> Result<usize> { match &self { ZstdCompression::Default(c, _) => c .decompress_to_buffer(source, destination) - .map_err(|e| into_error(e, "decompress header")), + .map_err(|e| into_error(e, "decompress header")), ZstdCompression::WithDict(c) => c .decompress_to_buffer(source, destination) - .map_err(|e| into_error(e, "decompress header")), + .map_err(|e| into_error(e, "decompress header")), } } }Note: The implementation above maintains the current behavior while introducing the helper method, but doesn't actually use it yet. You would need to refactor the method calls to use this helper.
99-99: Fix typo in comment.There's a small typo in the comment describing the
ZstdCompressionenum.-// Wrapper type to unify compressing with and withuot a dictionary, +// Wrapper type to unify compressing with and without a dictionary,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bongonet-header-serde/src/lib.rs(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
bongonet-header-serde/src/lib.rs (5)
29-30: Package imports look good!The imports now explicitly reference external packages
bongonet_errorandbongonet_http. This improves clarity and suggests proper modularization of the codebase.
40-40: Great refactoring of the compression field type.Changing the type to use the new
ZstdCompressionenum provides a cleaner abstraction for handling different compression strategies.
54-70: Initialization logic looks correct.The updated initialization properly uses the new enum variants based on dictionary presence. The conditional logic is clear and maintains the original functionality.
83-83: Simplified serialization call.Using the unified
compressmethod from theZstdCompressionenum improves code clarity by encapsulating the compression details within the enum implementation.
93-94: Cleaner error handling in deserialization.The updated code properly delegates error handling to the
ZstdCompressionenum implementation, making the code more concise and maintainable.
Signed-off-by: NxPKG <[email protected]>
Signed-off-by: NxPKG <[email protected]>
User description
Notes for Reviewers
This PR fixes #
Signed commits
PR Type
enhancement, configuration changes
Description
Added a new
clippy.tomlconfiguration file.Specified the Minimum Supported Rust Version (MSRV) as
1.72.Addressed the unmaintained status of the
ringpackage indirectly by ensuring compatibility with updated tooling.Changes walkthrough 📝
clippy.toml
Added Clippy configuration with MSRV set to 1.72clippy.toml
clippy.tomlconfiguration file.1.72for Clippy.Summary by CodeRabbit
Chores
New Features
ZstdCompressionenum, enhancing the clarity and structure of compression operations.ZstdCompressionenum, streamlining data handling processes.