Conversation
Reviewer's GuideThis PR updates project documentation to reflect completed AST deserialisation work, add testing details for the new parsing logic, and clean up markdown lint issues. 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 6 minutes and 41 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 (4)
Summary by CodeRabbit
WalkthroughUpdate documentation by removing a specific citation and adjusting formatting in behavioural testing docs, add a paragraph to the design document about unit and behavioural tests for manifest deserialization, and mark a roadmap checklist item as completed to reflect the finished implementation of AST struct annotations. 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 (
|
![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:
- Please verify that the AST structs in src/ast.rs have actually been annotated with #[derive(Deserialize)] and #[serde(deny_unknown_fields)] as the roadmap and docs now claim.
- Consider extracting the new testing details in netsuke-design.md into a dedicated Testing section to keep the design narrative focused and improve readability.
- It could be helpful to include a brief YAML example and its resulting AST in the docs to illustrate the new deserialization behavior concretely.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Please verify that the AST structs in src/ast.rs have actually been annotated with #[derive(Deserialize)] and #[serde(deny_unknown_fields)] as the roadmap and docs now claim.
- Consider extracting the new testing details in netsuke-design.md into a dedicated Testing section to keep the design narrative focused and improve readability.
- It could be helpful to include a brief YAML example and its resulting AST in the docs to illustrate the new deserialization behavior concretely.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
|
@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 (3)
docs/behavioural-testing-in-rust-with-cucumber.md(2 hunks)docs/netsuke-design.md(1 hunks)docs/roadmap.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🪛 LanguageTool
docs/behavioural-testing-in-rust-with-cucumber.md
[typographical] ~1115-~1115: To join two clauses or introduce examples, consider using an em dash.
Context: ...the-common-pitfalls> [^23]: Data tables - Cucumber Rust Book, accessed on 14 July ...
(DASH_RULE)
docs/netsuke-design.md
[style] ~553-~553: Would you like to use the Oxford spelling “deserialization”? The spelling ‘deserialisation’ is also correct.
Context: ...features/manifest.feature` validate the deserialisation logic. They verify that unknown fields ...
(OXFORD_SPELLING_Z_NOT_S)
🔇 Additional comments (1)
docs/roadmap.md (1)
23-25: Bullet wraps and spelling already comply with guidelinesThe new checklist entry sits within 80-column width, uses en-oxendic “serialize/deserialize” spelling, and follows existing bullet formatting. No action required.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 (3)
README.md(1 hunks)docs/netsuke-design.md(2 hunks)tests/steps/manifest_steps.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
**/*.md
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🪛 LanguageTool
docs/netsuke-design.md
[style] ~598-~598: Would you like to use the Oxford spelling “deserialization”? The spelling ‘deserialisation’ is also correct.
Context: ...features/manifest.feature` exercise the deserialisation logic. They assert that manifests fail ...
(OXFORD_SPELLING_Z_NOT_S)
[uncategorized] ~599-~599: Possible missing comma found.
Context: ...s fail to parse when unknown fields are present and that a minimal manifest round-trips...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (4)
README.md (2)
3-4: Opening tagline looks goodThe re-split tagline stays within the 80-column limit and avoids forbidden pronouns.
No further action needed.
9-10: Sentence merge is clear and compliantThe single-sentence description respects the 80-column rule and keeps wording free of 1st/2nd-person pronouns.
All good.docs/netsuke-design.md (1)
496-537: Example section is well-structuredThe new illustrative manifest and AST snippet improves clarity, stays within style rules, and keeps code blocks < 120 cols.
No issues.tests/steps/manifest_steps.rs (1)
8-11: Attribute tidy-up approvedRemoval of the trailing comma silences clippy without altering intent.
Looks great.
leynos
force-pushed
the
codex/add-serde_yml-parsing-to-ast-structs
branch
from
008f08f to
ba4c5d4
Compare
1 participant
Summary
Testing
make lintmake testmake markdownlintmake nixiehttps://chatgpt.com/codex/tasks/task_e_687b77a4275c83228d546d99e95772ac
Summary by Sourcery
Mark AST deserialization support as complete and update documentation accordingly, including test coverage details and cleanup of markdown footnotes.
Enhancements:
Documentation: