Cleanups for later refactor #58
Conversation
Reviewer's GuideThis PR applies cleanup changes in preparation for a larger refactor by simplifying test assertions in StreamParser, refining formatting macros in Display implementations, adjusting lifetimes in trait impls, enhancing build script annotations, and cleaning up dependencies and obsolete tests. Class diagram for updated Display implementationsclassDiagram
class ParseError {
+Display::fmt(&self, f: &mut Formatter) -> Result
}
class JsonNumber {
+Display::fmt(&self, f: &mut Formatter) -> Result
}
ParseError <|.. Display
JsonNumber <|.. Display
Class diagram for ChunkReader trait implementation changeclassDiagram
class ChunkReader {
+read(&mut self, buf: &mut [u8]) -> Result<usize, Self::Error>
}
class Reader {
<<trait>>
+read(&mut self, buf: &mut [u8]) -> Result<usize, Self::Error>
}
ChunkReader ..|> Reader
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis update removes unused dependencies from Cargo.toml, modernizes Rust formatting syntax throughout the codebase, adds or adjusts lint suppression attributes, and simplifies test code. It also deletes two debug test files. No logic or control flow is altered; all changes are stylistic, syntactic, or related to test code cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Cargo
participant picojson
Developer->>Cargo: Remove log and test-log dependencies
Developer->>picojson: Update formatting macros and lint attributes
Developer->>picojson: Simplify and delete debug test code
Cargo-->>Developer: Build with updated dependencies
picojson-->>Developer: Run with modernized code and cleaner test suite
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)warning: failed to write cache, path: /usr/local/registry/index/index.crates.io-1949cf8c6b5b557f/.cache/pa/st/paste, error: Permission denied (os error 13) error: could not compile 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @kaidokert, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on a series of targeted cleanups and minor refactorings across the picojson crate. The primary goal is to prepare the codebase for future, more significant refactoring efforts by improving code clarity, modernizing syntax, and removing unnecessary elements like unused dependencies and temporary debug tests. These changes aim to reduce technical debt and make the codebase easier to maintain and extend.
Highlights
- Dependency Cleanup: I've removed the
logcrate from direct dependencies andtest-logfrom dev-dependencies inCargo.toml, streamlining the project's dependency graph. - Code Modernization and Linting: I've updated several files (
build.rs,src/chunk_reader.rs,src/json_number.rs,src/parse_error.rs) to use more modern Rust syntax, such as Rust 2021 edition format strings ({val}instead of{},val), and applied#[allow(dead_code)]to utility functions inbuild.rsto suppress warnings for functions that are conditionally used or called indirectly. I also adjusted a module-level lint allowance inbuild.rs. - Test Code Refinement: I've simplified and cleaned up test assertions in
src/stream_parser.rsby replacing verbosematchstatements with conciseassert!(matches!()). Additionally, two temporary debug test files (tests/debug_root_numbers.rsandtests/debug_stress_case.rs) have been removed, as they are no longer needed.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @kaidokert - I've reviewed your changes - here's some feedback:
- Consider adding custom messages to your assert! statements so failures indicate which JSON event or buffer size caused the error.
- Instead of sprinkling allow(dead_code) on individual functions in build.rs, consider refactoring or removing unused helpers to keep the codebase clean.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding custom messages to your assert! statements so failures indicate which JSON event or buffer size caused the error.
- Instead of sprinkling allow(dead_code) on individual functions in build.rs, consider refactoring or removing unused helpers to keep the codebase clean.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Clean up code style and test harness by replacing print statements with assertions, modernize formatting, apply lint allowances, refine trait impl signatures, and remove unused dependencies to prepare for a later refactor
Enhancements:
{}->{var}) infmt::DisplayimplementationsChunkReaderby specifying a concrete lifetimeallow(non_snake_case)andallow(dead_code)) in generated build scriptBuild:
Tests:
stream_parserChores:
test-logandlogdependencies from Cargo.tomlSummary by CodeRabbit
Style
Tests
Chores