Conversation
Reviewer's GuideThis PR relaxes strict lint enforcement by commenting out select Clippy deny rules in Cargo.toml, ensures lint/validation commands in the Makefile are properly quoted, introduces documentation for the Jinja-based Class diagram for relaxed Clippy lint rules in Cargo.tomlclassDiagram
class ClippyLints {
- allow_attributes: deny (commented out)
- dbg_macro: deny (commented out)
- print_stdout: deny (commented out)
- print_stderr: deny (commented out)
- expect_used: deny (commented out)
allow_attributes_without_reason: deny
blanket_clippy_restriction_lints: deny
unwrap_used: deny
indexing_slicing: deny
string_slice: deny
integer_division: deny
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughComment out several Clippy lint rules in Changes
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 (
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Verify that the updated Markdown tables render correctly—several cells look truncated (e.g. “rules: {outs}”) and may need fixing or escaping.
- Consider using Markdown’s auto‐numbering (all items “1.”) instead of manual list numbers to avoid mismatches when sections are reordered.
- Add a brief explanation in Cargo.toml for why specific Clippy deny rules are commented out and when they’ll be re-enabled.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Verify that the updated Markdown tables render correctly—several cells look truncated (e.g. “rules: {outs}”) and may need fixing or escaping.
- Consider using Markdown’s auto‐numbering (all items “1.”) instead of manual list numbers to avoid mismatches when sections are reordered.
- Add a brief explanation in Cargo.toml for why specific Clippy deny rules are commented out and when they’ll be re-enabled.
## Individual Comments
### Comment 1
<location> `Makefile:36` </location>
<code_context>
markdownlint: ## Lint Markdown files
- find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 $(MDLINT)
+ find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- '$(MDLINT)'
nixie: ## Validate Mermaid diagrams
</code_context>
<issue_to_address>
Quoting the MDLINT variable in xargs may cause issues if it contains multiple arguments.
Quoting '$(MDLINT)' passes it as a single argument. Remove the quotes if MDLINT contains multiple arguments or commands.
</issue_to_address>
### Comment 2
<location> `Makefile:39` </location>
<code_context>
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)'
help: ## Show available targets
</code_context>
<issue_to_address>
Quoting the NIXIE variable in xargs may cause similar argument-passing issues.
Quoting '$(NIXIE)' causes it to be treated as a single argument, which may break if NIXIE includes multiple arguments. Remove the quotes unless this is the intended behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Cargo.toml(1 hunks)docs/netsuke-design.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
Cargo.toml
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🪛 markdownlint-cli2 (0.17.2)
docs/netsuke-design.md
1122-1122: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
🔇 Additional comments (1)
Cargo.toml (1)
23-24: Run Clippy with--deny clippy::expect_usedClippy failed in the sandbox due to a missing
/procmount. Run locally:cargo clippy --all-targets --all-features -- --deny clippy::expect_usedEnsure zero
expectusages remain or add targeted
#[allow(clippy::expect_used)]annotations with clear justifications.Locations to verify:
- Cargo.toml (lines 23–24):
# expect_used = "deny" indexing_slicing = "deny"
|
|
||
| This table serves as a quick-reference guide to the core third-party crates | ||
|
|
||
| <!-- markdownlint-disable MD013 MD033 --> | ||
|
|
||
| This table serves as a quick-reference guide to the core third-party crates | ||
| selected for this project and the rationale for their inclusion. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove stray trailing space to satisfy MD009
A single trailing space sneaks in at the start of the new table block (flagged by markdownlint). Strip it to keep the doc lint-clean.
-This table serves as a quick-reference guide to the core third-party crates␠
+This table serves as a quick-reference guide to the core third-party crates📝 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.
| This table serves as a quick-reference guide to the core third-party crates | |
| <!-- markdownlint-disable MD013 MD033 --> | |
| This table serves as a quick-reference guide to the core third-party crates | |
| selected for this project and the rationale for their inclusion. | |
| This table serves as a quick-reference guide to the core third-party crates | |
| selected for this project and the rationale for their inclusion. |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1122-1122: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
🤖 Prompt for AI Agents
In docs/netsuke-design.md around lines 1121 to 1123, there is a stray trailing
space at the start of the new table block causing a markdownlint MD009 warning.
Remove the trailing space before the table to ensure the markdown is lint-clean
and properly formatted.
|
@sourcery-ai resolve |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Please reference an issue or add more context for the temporarily disabled Clippy rules so it’s clear when and why they should be re-enabled.
- Consider adding a minimal example of the
foreachtarget usage in the docs or README to illustrate how loop-generated build targets look in practice. - It might be helpful to add a short comment in the Makefile explaining why the
markdownlintandnixiecommands need to be quoted for correct argument parsing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Please reference an issue or add more context for the temporarily disabled Clippy rules so it’s clear when and why they should be re-enabled.
- Consider adding a minimal example of the `foreach` target usage in the docs or README to illustrate how loop-generated build targets look in practice.
- It might be helpful to add a short comment in the Makefile explaining why the `markdownlint` and `nixie` commands need to be quoted for correct argument parsing.
## Individual Comments
### Comment 1
<location> `Cargo.toml:27` </location>
<code_context>
# 3. panic-prone operations
unwrap_used = "deny"
-expect_used = "deny"
+# Expect used in tests. Discoraged in implementation code.
+# expect_used = "deny"
indexing_slicing = "deny"
</code_context>
<issue_to_address>
Typo in the word 'Discoraged'.
Please change 'Discoraged' to 'Discouraged'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
# Expect used in tests. Discoraged in implementation code.
=======
# Expect used in tests. Discouraged in implementation code.
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # 3. panic-prone operations | ||
| unwrap_used = "deny" | ||
| expect_used = "deny" | ||
| # Expect used in tests. Discoraged in implementation code. |
There was a problem hiding this comment.
nitpick (typo): Typo in the word 'Discoraged'.
Please change 'Discoraged' to 'Discouraged'.
| # Expect used in tests. Discoraged in implementation code. | |
| # Expect used in tests. Discouraged in implementation code. |
Summary
foreachTesting
make fmtmake lintmake testmake markdownlintmake nixiehttps://chatgpt.com/codex/tasks/task_e_687264add674832291a5b756ab521e74
Summary by Sourcery
Comment out problematic Clippy deny rules in Cargo.toml, remove markdownlint directives around tables in the design documentation, and refine the phrasing of a table introduction.
Enhancements:
Documentation: