Align ninja_env crate with lint policy#118
Conversation
Reviewer's GuideThis PR marks the Class diagram for updated ninja_env constant usage in testsclassDiagram
class NinjaEnv {
<<constant>> NINJA_ENV : &str
}
class TestSupportEnv {
+uses NinjaEnv::NINJA_ENV
+uses OS-agnostic temp paths
+lossless Unicode handling
}
TestSupportEnv --> NinjaEnv : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider using tempfile::TempDir in your tests instead of std::env::temp_dir() to ensure unique, isolated directories and avoid collisions even with serial execution.
- The docs example still uses to_string_lossy() for path assertions; consider switching to to_str() (with proper error handling) for consistency with the test code’s lossless Unicode handling.
- Since this clippy policy is quite extensive and applies to multiple internal crates, you might centralize it in the workspace root to avoid duplication and ease future updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using tempfile::TempDir in your tests instead of std::env::temp_dir() to ensure unique, isolated directories and avoid collisions even with serial execution.
- The docs example still uses to_string_lossy() for path assertions; consider switching to to_str() (with proper error handling) for consistency with the test code’s lossless Unicode handling.
- Since this clippy policy is quite extensive and applies to multiple internal crates, you might centralize it in the workspace root to avoid duplication and ease future updates.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
d5838ca
into
codex/add-envlock-and-mockable-env-for-ninja_env
* Add guard for NINJA_ENV overrides * Quote NINJA_ENV in doc comment * Centralise NINJA_ENV and cover unset restoration * Remove manual NINJA_ENV cleanup in tests * Add crate for shared NINJA_ENV constant * Scope EnvLock in NINJA_ENV tests (#117) * Apply review suggestions for ninja env crate (#118) * Document NINJA_ENV and restore via OsString (#120) * Refine ninja env docs and tests (#121)
Summary
ninja_envcrate as internal and apply clippy lint policyninja_env::NINJA_ENVconstant and OS-agnostic temp paths in testsTesting
make fmtmake check-fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_689e8fb5ed2c83229b852efb9088dbc8
Summary by Sourcery
Align the ninja_env crate with the internal lint policy and improve its tests and examples
Enhancements:
Documentation:
Tests: