Conversation
Reviewer's GuideThis PR enriches the Netsuke design documentation by introducing a loop-based target generation section ( File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughCorrect indentation in the Makefile by replacing leading spaces with tabs for command lines under several targets. Update the documentation to fix numbering issues, improve table formatting, clarify CLI attribute spacing, and add a new section describing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BuildSystem
participant Manifest
participant TargetGenerator
User->>BuildSystem: Initiate build
BuildSystem->>Manifest: Parse manifest
Manifest->>TargetGenerator: Detect 'foreach' in targets
TargetGenerator->>Manifest: Expand targets using 'foreach'
Manifest->>BuildSystem: Return expanded build graph
BuildSystem->>User: Execute build with generated targets
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. 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 (
|
![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 - here's some feedback:
- The new pipe-style Markdown tables include complex escape sequences and multiline code snippets that may not render correctly; consider isolating those examples in separate fenced code blocks or reverting to HTML tables for better control.
- Several nested lists have repeated numbering (e.g., multiple '2.' entries) making the sequence hard to follow; switching to Markdown auto-numbering or bullet lists could improve clarity.
- In the new foreach-based target generation section, consider adding a note on how users can preview or debug the expanded targets (e.g., via a verbose flag) to make the feature more user-friendly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new pipe-style Markdown tables include complex escape sequences and multiline code snippets that may not render correctly; consider isolating those examples in separate fenced code blocks or reverting to HTML tables for better control.
- Several nested lists have repeated numbering (e.g., multiple '2.' entries) making the sequence hard to follow; switching to Markdown auto-numbering or bullet lists could improve clarity.
- In the new foreach-based target generation section, consider adding a note on how users can preview or debug the expanded targets (e.g., via a verbose flag) to make the feature more user-friendly.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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Makefile(1 hunks)docs/netsuke-design.md(26 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/netsuke-design.md (2)
Learnt from: CR
PR: leynos/netsuke#0
File: docs/netsuke-design.md:0-0
Timestamp: 2025-07-12T13:36:08.535Z
Learning: Applies to docs/src/ast.rs : Fields in the AST that can be either a single string or a list of strings must use an enum with #[serde(untagged)] (e.g., StringOrList).
Learnt from: CR
PR: leynos/netsuke#0
File: docs/netsuke-design.md:0-0
Timestamp: 2025-07-12T13:36:08.535Z
Learning: Netsuke must use Rust's std::process::Command API to invoke the 'ninja' process, forwarding CLI arguments and configuring working directory and I/O streams as needed.
🪛 LanguageTool
docs/netsuke-design.md
[uncategorized] ~58-~58: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ... Rust data structures. into a set of strongly-typed Rust data structures. This collection o...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~219-~219: Possible missing comma found.
Context: ...his evaluated during the Jinja render phase and each value becomesitem` in the ta...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~896-~896: Consider using the typographical ellipsis character here instead.
Context: ...nction where the error occurred returns Err(IrGenError::RuleNotFound {... }.into()). The .into() call converts...
(ELLIPSIS)
[style] ~1053-~1053: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile generator (ninja_gen.rs). 5. Implement the std::process::Command logic to in...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~1075-~1075: Consider using the typographical ellipsis character here instead.
Context: ... 4. Implement basic Jinja control flow ({% if... %}, {% for... %}) and variabl...
(ELLIPSIS)
[style] ~1075-~1075: Consider using the typographical ellipsis character here instead.
Context: ...asic Jinja control flow ({% if... %}, {% for... %}) and variable substitution. ...
(ELLIPSIS)
[typographical] ~1154-~1154: To join two clauses or introduce examples, consider using an em dash.
Context: ...ja-build.org/> 02. Ninja (build system) - Wikipedia, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1157-~1157: To join two clauses or introduce examples, consider using an em dash.
Context: ...Complete Guide To The Ninja Build System - Spectra - Mathpix, accessed on July ...
(DASH_RULE)
[typographical] ~1157-~1157: To join two clauses or introduce examples, consider using an em dash.
Context: ...uide To The Ninja Build System - Spectra - Mathpix, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1161-~1161: To join two clauses or introduce examples, consider using an em dash.
Context: ...e-to-the-ninja-build-system> 04. semver - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1165-~1165: To join two clauses or introduce examples, consider using an em dash.
Context: ...argo's flavor of Semantic Versioning - GitHub, accessed on July 12, 2025, <...
(DASH_RULE)
[typographical] ~1168-~1168: To join two clauses or introduce examples, consider using an em dash.
Context: ...//github.com/dtolnay/semver> 06. semver - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1168-~1168: To join two clauses or introduce examples, consider using an em dash.
Context: ...b.com/dtolnay/semver> 06. semver - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1171-~1171: To join two clauses or introduce examples, consider using an em dash.
Context: ...ver/latest/semver/> 07. How Ninja works - Fuchsia, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1177-~1177: To join two clauses or introduce examples, consider using an em dash.
Context: ...l.html> 09. Interest in new deps format - Google Groups, accessed on July 12, 2025...
(DASH_RULE)
[typographical] ~1182-~1182: To join two clauses or introduce examples, consider using an em dash.
Context: ...https://serde.rs/> 11. Saphyr libraries - crates.io: Rust Pack...
(DASH_RULE)
[typographical] ~1185-~1185: To join two clauses or introduce examples, consider using an em dash.
Context: ....io/crates/saphyr> 12. Saphyr libraries - A set of crates dedicated to parsing YAM...
(DASH_RULE)
[typographical] ~1185-~1185: Consider using an em dash in dialogues and enumerations.
Context: ...et of crates dedicated to parsing YAML. - GitHub, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1188-~1188: To join two clauses or introduce examples, consider using an em dash.
Context: ....com/saphyr-rs/saphyr> 13. saphyr-serde - crates.io: Rust Pack...
(DASH_RULE)
[typographical] ~1191-~1191: To join two clauses or introduce examples, consider using an em dash.
Context: ...ates.io/crates/saphyr-serde> 14. saphyr - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1191-~1191: To join two clauses or introduce examples, consider using an em dash.
Context: .../crates/saphyr-serde> 14. saphyr - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1194-~1194: To join two clauses or introduce examples, consider using an em dash.
Context: ... https://docs.rs/saphyr 15. minijinja - crates.io: Rust Pack...
(DASH_RULE)
[typographical] ~1197-~1197: To join two clauses or introduce examples, consider using an em dash.
Context: ...ates.io/crates/minijinja> 16. minijinja - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1197-~1197: To join two clauses or introduce examples, consider using an em dash.
Context: .../crates/minijinja> 16. minijinja - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1200-~1200: To join two clauses or introduce examples, consider using an em dash.
Context: ...tps://docs.rs/minijinja/> 17. minijinja - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1206-~1206: To join two clauses or introduce examples, consider using an em dash.
Context: ...plate-engine> 19. std::process::Command - Rust - MIT, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1206-~1206: To join two clauses or introduce examples, consider using an em dash.
Context: ...ngine> 19. std::process::Command - Rust - MIT, accessed on July 12, 2025, <htt...
(DASH_RULE)
[typographical] ~1212-~1212: Consider using an em dash in dialogues and enumerations.
Context: ...ion/> 21. How to check python version? - 4Geeks, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1215-~1215: To join two clauses or introduce examples, consider using an em dash.
Context: ...o-check-python-version> 22. shell_quote - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1215-~1215: To join two clauses or introduce examples, consider using an em dash.
Context: ...-python-version> 22. shell_quote - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1218-~1218: To join two clauses or introduce examples, consider using an em dash.
Context: ...e/latest/shell_quote/> 23. std::process - Rust - MIT, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1218-~1218: To join two clauses or introduce examples, consider using an em dash.
Context: ...t/shell_quote/> 23. std::process - Rust - MIT, accessed on July 12, 2025, <htt...
(DASH_RULE)
[typographical] ~1221-~1221: To join two clauses or introduce examples, consider using an em dash.
Context: ...td/process/index.html> 24. std::process - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1224-~1224: To join two clauses or introduce examples, consider using an em dash.
Context: ...index.html> 25. Command in std::process - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1227-~1227: To join two clauses or introduce examples, consider using an em dash.
Context: .../process/struct.Command.html> 26. shlex - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1227-~1227: To join two clauses or introduce examples, consider using an em dash.
Context: ...s/struct.Command.html> 26. shlex - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1235-~1235: To join two clauses or introduce examples, consider using an em dash.
Context: ...s, Examples, and Advanced Techniques - Technorely, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1238-~1238: To join two clauses or introduce examples, consider using an em dash.
Context: ...ractical guide to Error Handling in Rust - Dev State, accessed on July 12, 2025...
(DASH_RULE)
[typographical] ~1241-~1241: To join two clauses or introduce examples, consider using an em dash.
Context: ...ror_handling/> 30. thiserror and anyhow - Comprehensive Rust, accessed on July 12,...
(DASH_RULE)
[typographical] ~1244-~1244: To join two clauses or introduce examples, consider using an em dash.
Context: ...r precondition/argument checking in Rust - Stack Overflow, accessed on July 12,...
(DASH_RULE)
🔇 Additional comments (1)
Makefile (1)
19-23: Indent commands with tabs, not spaces – good fix.The updated indentation adheres to GNU make’s requirement that recipe lines begin with a tab, preventing the classic “missing separator” error at runtime.
| find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(MDLINT) | ||
|
|
||
| nixie: ## Validate Mermaid diagrams | ||
| find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(NIXIE) | ||
| find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(NIXIE) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use -print0 | xargs -0 but also quote $(MDLINT)/$(NIXIE).
Wrap the variables in $(shell printf ...) or single quotes to avoid mis-parsing when they contain spaces (e.g. path with spaces).
- find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(MDLINT)
+ find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- "$(MDLINT)"Repeat for the nixie target.
🤖 Prompt for AI Agents
In the Makefile around lines 36 to 40, the commands using $(MDLINT) and $(NIXIE)
are not quoted, which can cause issues if these variables contain spaces. To fix
this, wrap $(MDLINT) and $(NIXIE) in single quotes or use $(shell printf ...) to
ensure they are properly quoted and parsed. Apply this quoting consistently in
both the main command and the nixie target command.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
28-33: Guardmdformat-allto avoid hard dependency on local toolingRunning
mdformat-all(and its--checkvariant) unconditionally breaks workflows on machines where the tool is absent. Add a presence check and emit a helpful message before invocation.- mdformat-all + @command -v mdformat-all >/dev/null || \ + { printf 'mdformat-all not found; install with pipx install mdformat\n' >&2; exit 1; } + mdformat-all ... - mdformat-all --check + @command -v mdformat-all >/dev/null || \ + { printf 'mdformat-all not found; install with pipx install mdformat\n' >&2; exit 1; } + mdformat-all --check
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Makefile(1 hunks)docs/netsuke-design.md(23 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/netsuke-design.md (1)
Learnt from: CR
PR: leynos/netsuke#0
File: docs/netsuke-design.md:0-0
Timestamp: 2025-07-12T13:36:08.546Z
Learning: Netsuke must use Rust's std::process::Command API to invoke the 'ninja' process, forwarding CLI arguments and configuring working directory and I/O streams as needed.
🪛 LanguageTool
docs/netsuke-design.md
[style] ~893-~893: Consider using the typographical ellipsis character here instead.
Context: ...nction where the error occurred returns Err(IrGenError::RuleNotFound {... }.into()). The .into() call converts...
(ELLIPSIS)
[style] ~1050-~1050: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile generator (ninja_gen.rs). 5. Implement the std::process::Command logic to in...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~1072-~1072: Consider using the typographical ellipsis character here instead.
Context: ... 4. Implement basic Jinja control flow ({% if... %}, {% for... %}) and variabl...
(ELLIPSIS)
[style] ~1072-~1072: Consider using the typographical ellipsis character here instead.
Context: ...asic Jinja control flow ({% if... %}, {% for... %}) and variable substitution. ...
(ELLIPSIS)
[typographical] ~1151-~1151: To join two clauses or introduce examples, consider using an em dash.
Context: ...ja-build.org/> 02. Ninja (build system) - Wikipedia, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1154-~1154: To join two clauses or introduce examples, consider using an em dash.
Context: ...Complete Guide To The Ninja Build System - Spectra - Mathpix, accessed on July ...
(DASH_RULE)
[typographical] ~1154-~1154: To join two clauses or introduce examples, consider using an em dash.
Context: ...uide To The Ninja Build System - Spectra - Mathpix, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1158-~1158: To join two clauses or introduce examples, consider using an em dash.
Context: ...e-to-the-ninja-build-system> 04. semver - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1162-~1162: To join two clauses or introduce examples, consider using an em dash.
Context: ...argo's flavor of Semantic Versioning - GitHub, accessed on July 12, 2025, <...
(DASH_RULE)
[typographical] ~1165-~1165: To join two clauses or introduce examples, consider using an em dash.
Context: ...//github.com/dtolnay/semver> 06. semver - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1165-~1165: To join two clauses or introduce examples, consider using an em dash.
Context: ...b.com/dtolnay/semver> 06. semver - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1168-~1168: To join two clauses or introduce examples, consider using an em dash.
Context: ...ver/latest/semver/> 07. How Ninja works - Fuchsia, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1174-~1174: To join two clauses or introduce examples, consider using an em dash.
Context: ...l.html> 09. Interest in new deps format - Google Groups, accessed on July 12, 2025...
(DASH_RULE)
[typographical] ~1179-~1179: To join two clauses or introduce examples, consider using an em dash.
Context: ...https://serde.rs/> 11. Saphyr libraries - crates.io: Rust Pack...
(DASH_RULE)
[typographical] ~1182-~1182: To join two clauses or introduce examples, consider using an em dash.
Context: ....io/crates/saphyr> 12. Saphyr libraries - A set of crates dedicated to parsing YAM...
(DASH_RULE)
[typographical] ~1182-~1182: Consider using an em dash in dialogues and enumerations.
Context: ...et of crates dedicated to parsing YAML. - GitHub, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1185-~1185: To join two clauses or introduce examples, consider using an em dash.
Context: ....com/saphyr-rs/saphyr> 13. saphyr-serde - crates.io: Rust Pack...
(DASH_RULE)
[typographical] ~1188-~1188: To join two clauses or introduce examples, consider using an em dash.
Context: ...ates.io/crates/saphyr-serde> 14. saphyr - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1188-~1188: To join two clauses or introduce examples, consider using an em dash.
Context: .../crates/saphyr-serde> 14. saphyr - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1191-~1191: To join two clauses or introduce examples, consider using an em dash.
Context: ... https://docs.rs/saphyr 15. minijinja - crates.io: Rust Pack...
(DASH_RULE)
[typographical] ~1194-~1194: To join two clauses or introduce examples, consider using an em dash.
Context: ...ates.io/crates/minijinja> 16. minijinja - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1194-~1194: To join two clauses or introduce examples, consider using an em dash.
Context: .../crates/minijinja> 16. minijinja - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1197-~1197: To join two clauses or introduce examples, consider using an em dash.
Context: ...tps://docs.rs/minijinja/> 17. minijinja - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1203-~1203: To join two clauses or introduce examples, consider using an em dash.
Context: ...plate-engine> 19. std::process::Command - Rust - MIT, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1203-~1203: To join two clauses or introduce examples, consider using an em dash.
Context: ...ngine> 19. std::process::Command - Rust - MIT, accessed on July 12, 2025, <htt...
(DASH_RULE)
[typographical] ~1209-~1209: Consider using an em dash in dialogues and enumerations.
Context: ...ion/> 21. How to check python version? - 4Geeks, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1212-~1212: To join two clauses or introduce examples, consider using an em dash.
Context: ...o-check-python-version> 22. shell_quote - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1212-~1212: To join two clauses or introduce examples, consider using an em dash.
Context: ...-python-version> 22. shell_quote - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1215-~1215: To join two clauses or introduce examples, consider using an em dash.
Context: ...e/latest/shell_quote/> 23. std::process - Rust - MIT, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1215-~1215: To join two clauses or introduce examples, consider using an em dash.
Context: ...t/shell_quote/> 23. std::process - Rust - MIT, accessed on July 12, 2025, <htt...
(DASH_RULE)
[typographical] ~1218-~1218: To join two clauses or introduce examples, consider using an em dash.
Context: ...td/process/index.html> 24. std::process - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1221-~1221: To join two clauses or introduce examples, consider using an em dash.
Context: ...index.html> 25. Command in std::process - Rust, accessed on July 12, 2025, <ht...
(DASH_RULE)
[typographical] ~1224-~1224: To join two clauses or introduce examples, consider using an em dash.
Context: .../process/struct.Command.html> 26. shlex - Rust - Docs.rs, access...
(DASH_RULE)
[typographical] ~1224-~1224: To join two clauses or introduce examples, consider using an em dash.
Context: ...s/struct.Command.html> 26. shlex - Rust - Docs.rs, accessed on J...
(DASH_RULE)
[typographical] ~1232-~1232: To join two clauses or introduce examples, consider using an em dash.
Context: ...s, Examples, and Advanced Techniques - Technorely, accessed on July 12, 2025, ...
(DASH_RULE)
[typographical] ~1235-~1235: To join two clauses or introduce examples, consider using an em dash.
Context: ...ractical guide to Error Handling in Rust - Dev State, accessed on July 12, 2025...
(DASH_RULE)
[typographical] ~1238-~1238: To join two clauses or introduce examples, consider using an em dash.
Context: ...ror_handling/> 30. thiserror and anyhow - Comprehensive Rust, accessed on July 12,...
(DASH_RULE)
[typographical] ~1241-~1241: To join two clauses or introduce examples, consider using an em dash.
Context: ...r precondition/argument checking in Rust - Stack Overflow, accessed on July 12,...
(DASH_RULE)
| Large sets of similar outputs can clutter a manifest when written individually. | ||
| Netsuke supports a `foreach` entry within `targets` to generate multiple outputs | ||
| succinctly. The expression assigned to `foreach` is evaluated during the Jinja | ||
| render phase, and each value becomes `item` in the target context. |
There was a problem hiding this comment.
Wrap paragraph lines to ≤ 80 columns to satisfy markdownlint
Line 216 exceeds the mandated 80-column wrap limit for ordinary text. This will fail the markdownlint target.
-Netsuke supports a `foreach` entry within `targets` to generate multiple outputs
+Netsuke supports a `foreach` entry within `targets` to generate multiple
+outputsRun mdformat-all after fixing to preserve consistent wrapping.
📝 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.
| Large sets of similar outputs can clutter a manifest when written individually. | |
| Netsuke supports a `foreach` entry within `targets` to generate multiple outputs | |
| succinctly. The expression assigned to `foreach` is evaluated during the Jinja | |
| render phase, and each value becomes `item` in the target context. | |
| Large sets of similar outputs can clutter a manifest when written individually. | |
| Netsuke supports a `foreach` entry within `targets` to generate multiple | |
| outputs | |
| succinctly. The expression assigned to `foreach` is evaluated during the Jinja | |
| render phase, and each value becomes `item` in the target context. |
🤖 Prompt for AI Agents
In docs/netsuke-design.md around lines 215 to 218, the paragraph lines exceed
the 80-column limit required by markdownlint. Reformat the text so that no line
is longer than 80 characters by inserting line breaks appropriately. After
adjusting the wrapping, run mdformat-all to maintain consistent formatting
throughout the document.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
Makefile (3)
28-29: Guardmdformat-allto avoid hard failure on missing tool.Previous review already flagged this; the command still runs unconditionally. Detect the binary first and emit a friendly hint.
- mdformat-all + @command -v mdformat-all >/dev/null || \ + { printf 'mdformat-all not found; install with pipx install mdformat\n' >&2; exit 1; } + mdformat-all
35-35: Quote$(MDLINT)to survive spaces in the command path.The variable may expand to something like
/usr/local/bin/markdownlint --config .mdlrc. Pass it toxargsas a single argument.-find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- $(MDLINT) +find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- "$(MDLINT)"
38-38: Apply the same quoting fix to$(NIXIE).-find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- $(NIXIE) +find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- "$(NIXIE)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Makefile(1 hunks)
🔇 Additional comments (2)
Makefile (2)
18-20: Treat warnings as errors – well done.Running tests under
-D warningsenforces code hygiene across the entire workspace.
32-32: Formatting-check target looks good.Verifying formatting in CI with
--checkis the right call.
| target/%/$(APP): ## Build binary in debug or release mode | ||
| $(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(APP) | ||
| $(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(APP) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Pattern-based release flag is fine but fragile.
$(findstring release,$(@)) works for the current tree, yet fails if a binary is ever named something-release. Guard with an explicit directory check for robustness.
- $(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(APP)
+ $(CARGO) build $(BUILD_JOBS) $(if $(filter target/release/%,$@),--release) --bin $(APP)🤖 Prompt for AI Agents
In the Makefile at lines 21 to 23, the release flag detection uses `$(findstring
release,$(@))`, which can mistakenly trigger if the binary name contains
"release". To fix this, replace the string search with an explicit check that
the target directory is exactly "release" before adding the --release flag. This
ensures the release build flag is only applied when the directory part of the
target path is "release", avoiding false positives from binary names.
…ance - Updated docs/execplans/3-11-1-cli-config-struct.md to align with ortho_config 0.8.0 semantics and migration rules. - Replaced ortho-config-users-guide.md with upstream 0.8.0 version, adding detailed guidance on derive macro usage, dependency architecture, new attributes, typed Clap defaults with `cli_default_as_absent`, and documentation metadata generation. - Clarified dependency and macro usage with ortho_config crate aliasing support. - Introduced best practices for maintaining compatibility and upgrading from 0.7.x to 0.8.0. - Enhanced examples for default handling and integration with cargo-orthohelp documentation tooling. - Focused on improving reliability and usability of config schema derivation and CLI integration. These documentation enhancements provide a comprehensive reference for implementing the roadmap item 3.11.1 and safely upgrading ortho_config dependency while preserving existing CLI and config behavior. Refs #3.11.1 roadmap, ortho_config 0.8.0 migration Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
…ance - Updated docs/execplans/3-11-1-cli-config-struct.md to align with ortho_config 0.8.0 semantics and migration rules. - Replaced ortho-config-users-guide.md with upstream 0.8.0 version, adding detailed guidance on derive macro usage, dependency architecture, new attributes, typed Clap defaults with `cli_default_as_absent`, and documentation metadata generation. - Clarified dependency and macro usage with ortho_config crate aliasing support. - Introduced best practices for maintaining compatibility and upgrading from 0.7.x to 0.8.0. - Enhanced examples for default handling and integration with cargo-orthohelp documentation tooling. - Focused on improving reliability and usability of config schema derivation and CLI integration. These documentation enhancements provide a comprehensive reference for implementing the roadmap item 3.11.1 and safely upgrading ortho_config dependency while preserving existing CLI and config behavior. Refs #3.11.1 roadmap, ortho_config 0.8.0 migration Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
1 participant
Summary
foreachTesting
make fmtmake lint(fails: pedantic priority & println!)make testhttps://chatgpt.com/codex/tasks/task_e_687264add674832291a5b756ab521e74
Summary by Sourcery
Add
foreach-based target generation and polish documentation and build scriptsNew Features:
foreachsupport in the manifest for loop-based target generationEnhancements:
serdeandclapin Rust codeBuild: