Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces unstructured stdout messages for the Clean and Graph subcommands with structured tracing.info! logs. Sequence diagram for Clean and Graph subcommand loggingsequenceDiagram
participant Runner
participant tracing
Runner->>tracing: info!("Clean requested")
Runner->>tracing: info!("Graph requested")
Class diagram for updated logging in subcommandsclassDiagram
class Runner {
+run(cli: &Cli) Result
}
class Commands {
<<enum>>
Clean
Graph
}
Runner --> Commands
class tracing {
+info!(...)
}
Runner ..> tracing : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughReplace two stdout Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs⚙️ CodeRabbit Configuration File
Files:
🔍 Additional research (Deepwiki)Summary of relevant context found:
Suggested reviewer checks:
Tools used:
🔇 Additional comments (2)
✨ 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 (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![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 replaces direct console output (println!) with structured logging (info!) for the Clean and Graph subcommands in the runner module. The change standardizes logging across the codebase by leveraging the existing tracing infrastructure that's already set up in the project.
The Clean and Graph subcommands are currently stub implementations that only log their invocation, making this an ideal place to adopt the established logging framework. The info! macro is already imported in the runner module, and the tracing system is properly initialized in main.rs with conditional debug-level logging based on the verbose flag. This change provides better control over log output and maintains consistency with the project's logging approach.
Important Files Changed
File Changes
| Filename | Score | Overview |
|---|---|---|
| src/runner.rs | 5/5 | Replaced println! calls with info! logging for Clean and Graph subcommands |
Confidence score: 5/5
- This PR is safe to merge with minimal risk
- Score reflects simple, well-aligned changes that use existing infrastructure correctly
- No files require special attention
1 file reviewed, no comments
![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(1 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 (2 servers)
Deepwiki:
-
Document [docs/roadmap.md] details that the "Use tracing for subcommand logs" PR corresponds to Phase 3: Production Polish, which includes CLI completeness with added subcommands like
cleanandgraphthat delegate to ninja commands and provide refined CLI feedback. This phase also integrates thetracingcrate for logging and improves error handling and security. Thecleanandgraphcommands are explicitly mentioned as part of this phase's CLI completeness efforts (docs/roadmap.md:155-170). -
The
src/runner.rsmodule is responsible for command execution logic, including handling CLI commands such asCleanandGraph(Module Organization). The PR changes insrc/runner.rsreplaceprintln!calls withtracing'sinfo!macro for these subcommands, aligning with the Phase 3 roadmap goal of improving CLI feedback and logging. -
The Command Line Interface documentation confirms that
CleanandGraphare primary commands handled by the CLI and executed viarunner::run()(Command Line Interface). TheCleancommand removes build artifacts by invokingninja -t clean, and theGraphcommand generates a DOT format dependency graph vianinja -t graph. The PR's logging changes improve informational output for these commands. -
The Developer Guide and Code Quality Standards documents specify that logging and error handling improvements in Phase 3 include adopting structured logging with crates like
tracingand enhancing CLI feedback. The PR's use ofinfo!macros for subcommand logs fits within these standards and the roadmap's production polish phase.
Context7:
-
Document /context7/rs-tracing: The
info!macro in thetracingcrate constructs an event at the info level, functioning similarly to theevent!macro. It supports various patterns for specifying name, target, parent, fields, and arguments, allowing flexible info-level logging. (Source: Rustinfo!Macro API Documentation, https://docs.rs/tracing/latest/src/tracing/macros.rs) -
Document /context7/rs-tracing: Usage examples of the
info!macro show how to log info-level events with structured fields, targets, and formatted messages, e.g.,info!("Graph requested")would log an informational event with that message. (Source: Rustinfo!Macro Usage Examples, https://docs.rs/tracing/latest/tracing/macro.info) -
Document /context7/rs-tracing: The
info!macro expands to anevent!macro call at the INFO level, supporting named events, targets, parents, and arbitrary fields or formatted messages. This macro is part of the structured, event-based diagnostic information collection framework in Rust. (Source: Rustinfo!Macro Definition, https://docs.rs/tracing/latest/tracing/macro.info)
⏰ 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 (1)
src/runner.rs (1)
93-95: Confirm that tracing subscriber already captures info-level logsTracing subscriber is initialised in src/main.rs (line 13) with
fmt().with_max_level(Level::DEBUG).init();This setup emits info! events by default. No changes required.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
tracingTesting
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_689897a468f48322a40409089384af7c
Summary by Sourcery
Enhancements: