Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR refactors handle_build to use a Cow for the build path, avoiding a clone when an emitted path is provided and passing the borrowed path directly to run_ninja. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor handle_build in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant handle_build
participant run_ninja
Caller->>handle_build: invoke handle_build(args)
alt args.emit is Some(path)
handle_build->>handle_build: build_path = Cow::Borrowed(path)
handle_build->>handle_build: write Ninja content to emitted path
else args.emit is None
handle_build->>handle_build: tmp_file = create NamedTempFile
handle_build->>handle_build: build_path = Cow::Borrowed(tmp_file.path())
handle_build->>tmp_file: keep ownership to extend lifetime
end
handle_build->>run_ninja: run_ninja(build_path.as_ref(), ...)
run_ninja->>handle_build: return result
handle_build->>handle_build: drop(tmp_file) // ensure temp file removed after run
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Poem
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/runner.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
src/runner.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/runner.rs
🔍 MCP Research (1 server)
Deepwiki:
-
The
run_ninjafunction insrc/runner.rsis updated to accept a&Pathreference for the build path instead of a&PathBuf, reflecting the change to useCow<Path>inhandle_build. This aligns with the ownership semantics and avoids unnecessary cloning. (Related PR #46) -
The netsuke project enforces strict code quality standards, including no cloning of paths when unnecessary, and uses comprehensive testing including unit tests, linting, formatting checks, and behavioral tests to validate changes like the one in
handle_build. (Code Quality Standards, Testing Framework) -
There are no reported bugs or incidents specifically related to the
handle_buildfunction or the use ofCow<Path>insrc/runner.rs. The change is a micro-optimization flagged during code review and is intended to improve efficiency without altering behavior. (Issue #84 discussion)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR refactors the handle_build function in src/runner.rs to use Cow<Path> (Copy on Write) for more efficient memory management when handling build file paths. The change eliminates unnecessary heap allocations by borrowing the emitted path when the --emit option is provided, instead of cloning it.
The refactoring addresses a micro-optimization opportunity identified in issue #84. When users specify an output path via --emit, the original code was unnecessarily cloning the PathBuf, even though the path already exists in the args structure and could be borrowed instead. The new implementation uses Cow<Path> to express mixed ownership semantics - borrowing when the path is provided externally (emit case) and borrowing from owned temporary files when needed.
The change maintains the same external behavior while improving memory efficiency. The code now separates the variable declarations to manage lifetimes explicitly, ensuring the temporary file remains alive for the duration of the Ninja invocation. This fits well with the codebase's focus on performance and efficient resource management, as evidenced by the comprehensive testing infrastructure and build optimization goals of the Netsuke build system.
Important Files Changed
Files Modified
| Filename | Score | Overview |
|---|---|---|
| src/runner.rs | 4/5 | Refactored handle_build function to use Cow for borrowing build paths instead of cloning |
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it's a focused optimization that maintains existing behavior
- Score reflects a well-executed micro-optimization with proper lifetime management and no behavioral changes
- No files require special attention as the change is isolated and straightforward
1 file reviewed, no comments
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/runner.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
src/runner.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/runner.rs
🔍 MCP Research (1 server)
Deepwiki:
-
The
handle_buildfunction manages temporary Ninja files usingNamedTempFileand ensures proper lifetime management by explicit drop after Ninja invocation, preventing premature deletion. (PR summary) -
The
handle_buildfunction's update to useCow<Path>and borrow the emitted path avoids cloning, which reduces allocations and improves ownership clarity in the build execution logic. (PR summary, Issue #84) -
The
run_ninjafunction is called with a&Pathreference obtained frombuild_path.as_ref(), reflecting the change to useCow<Path>for the build path. The temporary file is explicitly dropped after Ninja invocation to ensure its lifetime covers the build process. (PR summary)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (4)
src/runner.rs (4)
11-11: Add Cow import for mixed ownership semantics — LGTMImport
Cowto support borrowed/owned path semantics. Matches the PR objective and keeps the API surface unchanged.
118-135: Borrow emitted path and keep temp alive — correct, allocation-free, borrow-checker-safeBorrow the emitted path (
Cow::Borrowed) and keepNamedTempFilealive while borrowing its path. This removes theto_path_buf()clone and aligns with the optimisation and Deepwiki notes.
138-139: Pass borrowed path to run_ninja and drop temp after invocation — LGTMPass
build_path.as_ref()torun_ninjaand explicitly drop the temp file after the build. This satisfies the lifetime requirement noted in Deepwiki.
118-135: Resolve build-path clone concernsNo stray
to_path_buf()calls remain in thehandle_buildflow –build_pathis always aCow::Borrowed. Allrun_ninjainvocations receive a&Path. The only remainingto_path_buf()is at src/runner.rs:333 as a fallback forcanonicalize(), which is required to supply aPathBufon error. No action required.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/runner.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Comment why, not what. Explain assumptions, edge cases, trade-offs, or complexity. Don't echo the obvious.
Use functions and composition. Avoid repetition by extracting reusable logic. Prefer generators or comprehensions, and declarative code to imperative repetition when readable.
Small, meaningful functions. Functions must be small, clear in purpose, single responsibility, and obey command/query segregation.
Name things precisely. Use clear, descriptive variable and function names. For booleans, prefer names with is, has, or should.
Every module must begin with a module level (//! ) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
Do not use return in single-line functions.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Prefer .expect() over .unwrap().
Use concat!() to combine long string literals rather than escaping newlines with a backslash.
Prefer single line versions of functions where appropriate.
Clippy warnings MUST be disallowed.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Keep file size manageable. No single code file may be longer than 400 lines. Long switch statements or dispatch tables should be broken up by feature and constituents colocated with targets. Large blocks of test data should be moved to external data files.
Illustrate with clear examples. Function documentation must include clear examples demonstrating the usage and outcome of the function. Test documentation should omit examples where the example serves only to reiterate the test logic.
Prefer semantic error enums. Derive std::error::Error (via the thiserror crate) for any condition the caller might inspect, retry, or map to an HTTP status.
...
Files:
src/runner.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/runner.rs
🔍 MCP Research (1 server)
Deepwiki:
- DeepWiki (https://deepwiki.com/search/borrow-build-path-when-emittin_f5ee3eae-4621-4a8e-995b-bb755c0b030b): The codebase converts a Netsuke manifest into a BuildGraph in Stage 4 via BuildGraph::from_manifest, producing the IR used to generate Ninja files.
- DeepWiki (https://deepwiki.com/search/borrow-build-path-when-emittin_f5ee3eae-4621-4a8e-995b-bb755c0b030b): The process_targets function inside BuildGraph::from_manifest converts manifest target names/sources/order-only deps to PathBufs using a to_paths helper.
- DeepWiki (https://deepwiki.com/search/borrow-build-path-when-emittin_f5ee3eae-4621-4a8e-995b-bb755c0b030b): BuildEdge stores paths as Vec (owned PathBufs) within the BuildGraph structure.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
| use crate::{ir::BuildGraph, manifest, ninja_gen}; | ||
| use anyhow::{Context, Result}; | ||
| use serde_json; | ||
| use std::borrow::Cow; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Cow import no longer needed if you simplify to &Path
Drop this import once you replace Cow<Path> with &Path as suggested below.
- use std::borrow::Cow;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/runner.rs around line 11, the use std::borrow::Cow import is unnecessary
once you replace Cow<Path> with &Path; update function signatures and local
variables that currently use Cow<Path> to accept &Path instead, adjust any call
sites to pass a reference (and replace Cow-specific methods with &Path
equivalents like as_ref()/to_path_buf() or direct &), then remove the `use
std::borrow::Cow;` line; ensure lifetimes and mutable/ownership expectations are
satisfied by borrowing or cloning where required.
| // Normalize the build file path and keep the temporary file alive for the | ||
| // duration of the Ninja invocation. Borrow the emitted path when provided | ||
| // to avoid unnecessary allocation. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Correct the comment: path canonicalization happens in run_ninja, not here
Avoid misleading readers about where normalisation occurs; keep the focus on lifetimes and borrowing.
- // Normalize the build file path and keep the temporary file alive for the
- // duration of the Ninja invocation. Borrow the emitted path when provided
- // to avoid unnecessary allocation.
+ // Keep the temporary build file alive for the duration of the Ninja
+ // invocation. Borrow the emitted path when provided to avoid unnecessary
+ // allocation; run_ninja canonicalizes the path.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Normalize the build file path and keep the temporary file alive for the | |
| // duration of the Ninja invocation. Borrow the emitted path when provided | |
| // to avoid unnecessary allocation. | |
| // Keep the temporary build file alive for the duration of the Ninja | |
| // invocation. Borrow the emitted path when provided to avoid unnecessary | |
| // allocation; run_ninja canonicalizes the path. |
🤖 Prompt for AI Agents
In src/runner.rs around lines 118 to 120, the comment incorrectly states that
path canonicalization/normalization happens here; update the comment to remove
that claim and instead describe only keeping the temporary file alive for the
duration of the Ninja invocation and borrowing the emitted path to avoid
allocation, focusing on lifetimes/borrowing rather than normalization location.
| let build_path: Cow<Path>; | ||
| let mut tmp_file: Option<NamedTempFile> = None; | ||
| if let Some(path) = &args.emit { | ||
| write_ninja_file(path, &ninja)?; | ||
| (path.clone(), None) | ||
| build_path = Cow::Borrowed(path.as_path()); | ||
| } else { | ||
| let tmp = create_temp_ninja_file(&ninja)?; | ||
| (tmp.path().to_path_buf(), Some(tmp)) | ||
| }; | ||
| tmp_file = Some(tmp); | ||
| build_path = Cow::Borrowed( | ||
| tmp_file | ||
| .as_ref() | ||
| .expect("temporary Ninja file should exist") | ||
| .path(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify: replace Cow<Path> with &Path; the owned variant is never used
Both branches produce borrowed paths. Remove Cow to reduce indirection and imports while preserving lifetimes.
- let build_path: Cow<Path>;
+ let build_path: &Path;
let mut tmp_file: Option<NamedTempFile> = None;
if let Some(path) = &args.emit {
write_ninja_file(path, &ninja)?;
- build_path = Cow::Borrowed(path.as_path());
+ build_path = path.as_path();
} else {
let tmp = create_temp_ninja_file(&ninja)?;
tmp_file = Some(tmp);
- build_path = Cow::Borrowed(
- tmp_file
- .as_ref()
- .expect("temporary Ninja file should exist")
- .path(),
- );
+ build_path = tmp_file
+ .as_ref()
+ .expect("temporary Ninja file should exist")
+ .path();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let build_path: Cow<Path>; | |
| let mut tmp_file: Option<NamedTempFile> = None; | |
| if let Some(path) = &args.emit { | |
| write_ninja_file(path, &ninja)?; | |
| (path.clone(), None) | |
| build_path = Cow::Borrowed(path.as_path()); | |
| } else { | |
| let tmp = create_temp_ninja_file(&ninja)?; | |
| (tmp.path().to_path_buf(), Some(tmp)) | |
| }; | |
| tmp_file = Some(tmp); | |
| build_path = Cow::Borrowed( | |
| tmp_file | |
| .as_ref() | |
| .expect("temporary Ninja file should exist") | |
| .path(), | |
| ); | |
| } | |
| let build_path: &Path; | |
| let mut tmp_file: Option<NamedTempFile> = None; | |
| if let Some(path) = &args.emit { | |
| write_ninja_file(path, &ninja)?; | |
| build_path = path.as_path(); | |
| } else { | |
| let tmp = create_temp_ninja_file(&ninja)?; | |
| tmp_file = Some(tmp); | |
| build_path = tmp_file | |
| .as_ref() | |
| .expect("temporary Ninja file should exist") | |
| .path(); | |
| } |
🤖 Prompt for AI Agents
In src/runner.rs around lines 121 to 135, replace the Cow<Path> usage with a
borrowed &Path since neither branch ever produces an owned Path: change the
declaration to let build_path: &Path; keep tmp_file: Option<NamedTempFile>
declared before so its lifetime spans the borrow; in the if branch assign
build_path = path.as_path(); in the else branch assign build_path =
tmp_file.as_ref().expect("temporary Ninja file should exist").path(); and remove
the now-unused Cow import.
|
|
||
| let program = resolve_ninja_program(); | ||
| run_ninja(program.as_path(), cli, &build_path, &targets)?; | ||
| run_ninja(program.as_path(), cli, build_path.as_ref(), &targets)?; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Pass build_path directly after the simplification
If you adopt the &Path refactor above, remove the .as_ref() call here.
- run_ninja(program.as_path(), cli, build_path.as_ref(), &targets)?;
+ run_ninja(program.as_path(), cli, build_path, &targets)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run_ninja(program.as_path(), cli, build_path.as_ref(), &targets)?; | |
| run_ninja(program.as_path(), cli, build_path, &targets)?; |
🤖 Prompt for AI Agents
In src/runner.rs around line 138, the call currently uses build_path.as_ref();
remove the unnecessary .as_ref() and pass build_path directly (or as a borrowed
reference if needed, e.g. &build_path) so the argument matches the simplified
&Path type expected by run_ninja.
| let program = resolve_ninja_program(); | ||
| run_ninja(program.as_path(), cli, &build_path, &targets)?; | ||
| run_ninja(program.as_path(), cli, build_path.as_ref(), &targets)?; | ||
| drop(tmp_file); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove redundant explicit drop
tmp_file drops at end-of-scope immediately after this line anyway. Avoid noise.
- drop(tmp_file);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| drop(tmp_file); |
🤖 Prompt for AI Agents
In src/runner.rs around line 139, the explicit call drop(tmp_file); is redundant
because tmp_file will be dropped automatically at the end of the scope; remove
that line to avoid noise and rely on Rust's RAII for cleanup (ensure no code
depends on immediate drop timing before deleting).
1 participant
Summary
--emitis used by borrowing the emitted pathrun_ninjaTesting
make fmtmake check-fmtmake lintmake testcloses #84
https://chatgpt.com/codex/tasks/task_e_6896813806388322b8a8b5ffd8794566
Summary by Sourcery
Use Cow in handle_build to borrow emitted build paths and avoid unnecessary allocations when using the --emit option, and update the run_ninja invocation accordingly.
Enhancements: