docs: update packages design dock #1616
Conversation
derberg
requested review from
Florence-Njeri and
asyncapi-bot-eve
as code owners
|
WalkthroughThe guideline in the Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/README.md (1)
24-24: Fix capitalization and improve clarity of the testing guideline
The second sentence starts with a lowercase letter and could be merged for better readability.-6. Helpers need dedicated tests. Always push for having test cases presented through dummy AsyncAPI documents. mock fake Parser objects inside tests. +6. Helpers need dedicated tests. Always push for having test cases presented through dummy AsyncAPI documents, though mocking fake Parser objects inside tests is still permitted.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/README.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
|
nah, sorry, my VSCode had a hiccup and autosafe stopped working |
|
updated |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/README.md (2)
24-24: Clarify and correct test guideline wording.The sentence "Mock fake Parser objects inside tests only is special cases where using dummy document is not an option." is grammatically incorrect and hard to parse. Consider rephrasing for clarity:
“Helpers need dedicated tests. Always provide test cases using dummy AsyncAPI documents, and mock Parser objects only when dummy documents cannot be used.”
31-31: Fix grammatical agreement and improve clarity.Change the wording to address number agreement and readability:
“Templates are considered ready for use if they have acceptance tests configured with Microcks, which provides mocks and a runtime environment. An example implementation is available in tests for websocket.”
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/README.md
[style] ~27-~27: This phrase is redundant. Consider writing “consulted”.
Context: ...ew template feature is added it must be consulted with: - Spec references docs using [raw d...
(CONSULT_WITH)
[grammar] ~31-~31: Did you mean “are” or “were”?
Context: ...nt tests can be executed. 11. Templates is considered to be of a good quality and ...
(SENT_START_NNS_IS)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - windows-latest
derberg
changed the title
There was a problem hiding this comment.
Hey @derberg One small change request and it will be good to merge
Co-authored-by: Florence Njeri <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/README.md (1)
31-31: Fix grammar in the template readiness guideline.The phrase starts with “A templates is…” and misses a verb in the example sentence. A more grammatically correct version could be:
- “A template is considered of good quality and ready for use if it has acceptance tests configured using Microcks, which provides both mocks and runtime support. An example implementation is available at tests for websocket.”
🧹 Nitpick comments (1)
packages/README.md (1)
24-24: Clarify the test guidance grammar.The sentence “Mock fake Parser objects inside tests only is special cases where using dummy document is not an option” contains a typo and grammatical error. Consider rephrasing for clarity:
- “Mock fake Parser objects inside tests only in special cases where using dummy documents is not an option.”
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/README.md
[style] ~27-~27: This phrase is redundant. Consider writing “consulted”.
Context: ...ew template feature is added it must be consulted with: - Spec references docs using [raw d...
(CONSULT_WITH)
[uncategorized] ~30-~30: A comma might be missing here.
Context: ...s a lot of control on tests running. We can for example have templates:test scrip...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~30-~30: A comma might be missing here.
Context: ...of control on tests running. We can for example have templates:test script that will ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ... tests running. We can for example have templates:test script that will run tes...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[grammar] ~31-~31: The plural noun “templates” cannot be used with the article “A”. Did you mean “A template” or “templates”?
Context: ...the relevant tests can be executed. 11. A templates is considered to be of a good quality a...
(A_NNS)
[grammar] ~31-~31: You should probably use: “are”.
Context: ... tests can be executed. 11. A templates is considered to be of a good quality and ...
(AGREEMENT_SENT_START)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Acceptance tests for generated templates
- GitHub Check: Test generator as dependency with Node 18
- GitHub Check: Test generator as dependency with Node 20
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
|
|
/rtm |
Resolves #1610
Summary by CodeRabbit