Conversation
Reviewer's GuideExtract build subcommand arguments into a dedicated BuildArgs struct and propagate the new Commands::Build(BuildArgs) signature throughout the runner, documentation, and tests. Class diagram for refactored CLI command structureclassDiagram
Cli "1" -- "0..1" Commands : command
Commands <|-- BuildArgs
class Cli {
+Option<Commands> command
}
class Commands {
<<enum>>
+Build(BuildArgs)
+Clean
+Graph
+Manifest
}
class BuildArgs {
+Option<PathBuf> emit
+Vec<String> targets
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor the CLI command structure by extracting the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant BuildArgs
participant Runner
User->>CLI: Invoke build command with arguments
CLI->>BuildArgs: Parse build arguments into BuildArgs struct
CLI->>Runner: Pass Commands::Build(BuildArgs)
Runner->>BuildArgs: Access emit and targets fields
Runner-->>User: Execute build process and return result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)docs/**/*.md📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
**/*.md📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
⚙️ CodeRabbit Configuration File
Files:
🔇 Additional comments (4)
✨ 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
docs/netsuke-design.md(2 hunks)src/cli.rs(2 hunks)src/runner.rs(3 hunks)tests/cli_tests.rs(1 hunks)tests/runner_tests.rs(5 hunks)tests/steps/cli_steps.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
tests/cli_tests.rssrc/runner.rssrc/cli.rstests/runner_tests.rstests/steps/cli_steps.rs
**/*.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Paragraphs and bullets must be wrapped to 80 columns, except where a long URL would prevent this (in which case, silence MD013 for that line)
- Code blocks should be wrapped to 120 columns.
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/netsuke-design.md
🧬 Code Graph Analysis (1)
tests/runner_tests.rs (1)
tests/steps/cli_steps.rs (1)
emit_path(140-143)
🔇 Additional comments (18)
src/cli.rs (4)
6-6: Import addition looks correct.The
Argsimport is properly added to support the newBuildArgsstruct.
83-92: Well-structured argument encapsulation.The
BuildArgsstruct properly encapsulates build command arguments with appropriate derives and documentation. The field structure matches the previous inline implementation.
98-98: Clean enum variant refactor.The
Commands::Buildvariant now properly wrapsBuildArgs, maintaining the same functionality whilst improving modularity.
74-77: Default command construction updated correctly.The default command construction properly uses the new
BuildArgsstruct with appropriate field initialization.tests/cli_tests.rs (2)
7-7: Import updated appropriately.The
BuildArgsimport correctly supports the refactored test construction patterns.
12-28: Test cases properly updated for new structure.All
Commands::Buildconstructions correctly use theBuildArgswrapper whilst maintaining the same test coverage and assertions.tests/steps/cli_steps.rs (3)
9-9: Import addition supports refactored structure.The
BuildArgsimport correctly enables the updated cucumber step implementations.
19-22: Default command construction updated properly.The fallback default command correctly uses the
BuildArgsstruct with appropriate field initialization.
37-37: Pattern matching updated correctly.The destructuring pattern properly accesses the
BuildArgsfields through the struct wrapper, maintaining the same extraction logic.tests/runner_tests.rs (5)
1-1: Import updated to support refactored tests.The
BuildArgsimport correctly enables the updated test construction patterns throughout the file.
19-22: Test construction properly refactored.The
Commands::Buildconstruction correctly usesBuildArgswrapper whilst maintaining identical test behavior.
38-41: Consistent refactoring across test functions.All test functions properly adopt the
BuildArgsstructure, maintaining test integrity and coverage.
74-77: Build command construction remains functionally identical.The refactored construction maintains the same test scenarios whilst using the new
BuildArgsencapsulation.
114-117: Emit functionality test properly updated.The test correctly constructs
Commands::Buildwith the emit path specified in theBuildArgsstruct, maintaining the same test logic.src/runner.rs (4)
7-7: Import supports refactored CLI handling.The
BuildArgsimport correctly enables the updated command processing logic.
72-75: Default command fallback properly updated.The default command construction correctly uses the
BuildArgsstruct with appropriate field initialization.
77-80: Command processing logic correctly refactored.The pattern matching and field access properly adapt to the
BuildArgsstructure whilst maintaining identical runtime behavior.
143-143: Documentation example updated appropriately.The doc comment example correctly reflects the new
BuildArgsusage, maintaining accurate documentation.
1 participant
Summary
BuildArgsstruct and explicit--emitCommands::Build(BuildArgs)patternTesting
make fmtmake lintmake testmake markdownlint./target/debug/netsuke --file tests/data/minimal.yml --verbose build --emit /tmp/out.ninja hellohttps://chatgpt.com/codex/tasks/task_e_68930362753c832284a53f268ffc83bd
Summary by Sourcery
Refactor the build subcommand to use a dedicated BuildArgs struct and update corresponding CLI parsing, runner logic, tests, and documentation to match the new pattern.
Enhancements:
Documentation:
Tests: