test: add markdown tests #3301
test: add markdown tests #3301
Conversation
anshgoyalevil
requested review from
derberg,
akshatnema,
magicmatatjahu,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe pull request introduces enhancements to the Node.js pull request testing workflow. Key changes include the addition of a step for markdown checks, which executes a new script to validate markdown files and their frontmatter. If issues are found, a comment is posted on the pull request. Conversely, if no issues are detected, any existing comment is deleted. Additionally, a new script for markdown validation is added to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
asyncapi-bot
requested review from
devilkiller-ag,
sambhavgupta0705,
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3301--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
.github/workflows/if-nodejs-pr-testing.yml (2)
87-101: Approve: PR comment for markdown issues with suggestionThis step effectively addresses the PR objective of alerting the team when markdown files have issues. The use of a sticky comment is a good choice for maintaining visibility without cluttering the PR.
Consider adding a brief explanation of what the markdown check does and how to resolve common issues. This could be added to the comment message like so:
message: | ### Markdown Check Results + The markdown check validates the structure and front-matter of markdown files. + Common issues include incorrect YAML front-matter formatting or missing required fields. + We found issues in the following markdown files: ``` ${{ steps.markdown_check.outputs.markdown_output }} ``` + Please review and correct these files before merging.This additional context can help developers quickly understand and resolve the issues.
83-83: Minor: Remove trailing spacesThere are trailing spaces at the end of this line. While this doesn't affect functionality, it's generally good practice to remove them for cleaner code.
Please remove the trailing spaces from this line:
- OUTPUT=$(npm run test:md || true) + OUTPUT=$(npm run test:md || true)🧰 Tools
🪛 yamllint
[error] 83-83: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .github/workflows/if-nodejs-pr-testing.yml (2 hunks)
- package.json (1 hunks)
- scripts/markdown/check-markdown.js (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/if-nodejs-pr-testing.yml
82-82: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
🪛 yamllint
.github/workflows/if-nodejs-pr-testing.yml
[error] 83-83: trailing spaces
(trailing-spaces)
🪛 Biome
scripts/markdown/check-markdown.js
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 37-37: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (4)
.github/workflows/if-nodejs-pr-testing.yml (3)
64-64: LGTM: Consistent quote usageThe change from double quotes to single quotes for the
node-versioninput is a minor improvement in consistency. This doesn't affect functionality but contributes to better code style.
103-110: Approve: Cleanup step for markdown check commentsThis step is a great addition that complements the previous comment step. By removing the comment when no issues are found, it keeps the PR clean and focused. This is an excellent practice for maintaining a tidy and informative PR thread.
Line range hint
1-111: Summary: Effective implementation of markdown checks with minor improvements neededThe changes to this workflow file successfully implement the markdown checking functionality as outlined in the PR objectives. The new steps for running markdown checks, commenting on issues, and cleaning up comments work together to provide a robust system for ensuring markdown file quality.
Key points:
- The markdown checks are now integrated into the PR testing process.
- The workflow will alert the team by commenting on the PR when markdown files have issues, addressing the concern raised in issue Automation testing for MD files #3215.
- The cleanup step ensures that the PR remains tidy when no issues are found.
These changes will significantly improve the reliability of markdown file rendering and help prevent instances where markdown files don't render properly without notification.
To further improve the implementation:
- Update the deprecated
set-outputcommand as suggested earlier.- Consider adding more context to the PR comment to help developers quickly resolve issues.
- Remove the trailing spaces on line 83.
Once these minor issues are addressed, this implementation will provide a solid foundation for ongoing markdown file quality assurance.
🧰 Tools
🪛 actionlint
82-82: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
🪛 yamllint
[error] 83-83: trailing spaces
(trailing-spaces)
package.json (1)
26-26: New script for markdown testing looks good.The addition of the
"test:md"script aligns well with the PR objectives of implementing tests for markdown files. This script will run thecheck-markdown.jsfile, which is likely responsible for validating the gray matter (front-matter) in markdown files as mentioned in the PR summary.A few points to consider:
- Ensure that the
scripts/markdown/check-markdown.jsfile exists and contains the necessary logic for markdown validation.- Consider integrating this new script into your CI/CD pipeline to automate markdown checks on every pull request.
- You might want to update the main
"test"script to include this new markdown test, ensuring it runs alongside other tests.To verify the existence of the script file and its potential usage, let's run the following:
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/if-nodejs-pr-testing.yml (2)
78-85: Improve script robustnessThe new step for running markdown checks is a great addition and aligns well with the PR objectives. However, there's a minor improvement we can make to ensure robustness:
Consider quoting the
$ERRORSvariable to prevent potential issues with word splitting or globbing:- ERRORS=$(npm run test:md) - echo "markdown_output=$ERRORS" >> $GITHUB_OUTPUT + ERRORS=$(npm run test:md) + echo "markdown_output<<EOF" >> $GITHUB_OUTPUT + echo "$ERRORS" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUTThis change will ensure that the entire output is captured correctly, even if it contains spaces or special characters.
🧰 Tools
🪛 actionlint
82-82: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
[error] 83-83: trailing spaces
(trailing-spaces)
83-83: Remove trailing spacesThere are trailing spaces at the end of line 83. While this doesn't affect functionality, it's good practice to remove them for code cleanliness.
Please remove the trailing spaces from this line:
- ERRORS=$(npm run test:md) + ERRORS=$(npm run test:md)🧰 Tools
🪛 yamllint
[error] 83-83: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/if-nodejs-pr-testing.yml (2 hunks)
- scripts/markdown/check-markdown.js (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/if-nodejs-pr-testing.yml
82-82: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
.github/workflows/if-nodejs-pr-testing.yml
[error] 83-83: trailing spaces
(trailing-spaces)
🪛 Biome
scripts/markdown/check-markdown.js
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 37-37: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (3)
.github/workflows/if-nodejs-pr-testing.yml (3)
86-100: Excellent addition of PR commenting for markdown issuesThis new step effectively addresses the PR objective of alerting the team when markdown files fail to render correctly. The use of a sticky comment ensures that the issues remain visible and prevents cluttering the PR with multiple comments.
The implementation is well-structured and provides clear, formatted output of any markdown issues found. This will greatly improve the team's ability to identify and address rendering problems quickly.
102-109: Approve: Efficient cleanup of markdown check commentsThis step is a great complement to the previous one. It ensures that the PR remains clean and free of unnecessary comments when no markdown issues are found.
By removing outdated comments, this step helps prevent confusion and keeps the PR focused on current issues. The implementation is consistent with the previous step and uses the same action effectively.
Line range hint
1-110: Overall assessment: Excellent implementation of markdown checksThe changes made to this workflow file effectively address the PR objectives by implementing markdown checks and providing a clear mechanism for reporting issues. The use of sticky comments for both reporting issues and cleaning up resolved ones is a great approach.
While there are a few minor suggestions for improvement, the overall implementation is solid and will significantly enhance the team's ability to catch and address markdown rendering issues early in the development process.
🧰 Tools
🪛 actionlint
82-82: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint
[error] 83-83: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
.github/workflows/if-nodejs-pr-testing.yml (2)
78-86: Approve: New markdown check step with suggested improvementsThe addition of this step aligns well with the PR objective of implementing tests for markdown files. It's great to see that the deprecated
set-outputcommand has been replaced with the new syntax as suggested in the previous review.To further improve the script, consider addressing the following shellcheck suggestions:
- Use curly braces for command grouping:
- ERRORS=$(npm run test:md | sed -n '/Errors in file/,$p') - echo "markdown_output<<EOF" >> $GITHUB_OUTPUT - echo "$ERRORS" >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT + { + echo "markdown_output<<EOF" + npm run test:md | sed -n '/Errors in file/,$p' + echo "EOF" + } >> $GITHUB_OUTPUT
- Double quote variables to prevent globbing and word splitting:
- ERRORS=$(npm run test:md | sed -n '/Errors in file/,$p') + ERRORS="$(npm run test:md | sed -n '/Errors in file/,$p')"These changes will make the script more robust and align with shell scripting best practices.
🧰 Tools
🪛 actionlint
82-82: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:3:19: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:4:15: Double quote to prevent globbing and word splitting
(shellcheck)
78-111: Summary: Effective implementation of markdown checks with suggestions for next stepsThe changes in this PR successfully implement the markdown checking process, which directly addresses the main objective of automating tests for markdown files. The addition of steps to run checks, comment on issues, and clean up comments when no issues are found creates a robust workflow for maintaining markdown quality.
However, the Lighthouse report mentioned in the PR comments indicates some performance concerns:
- Performance: 41 (🔴)
- PWA: 33 (🔴)
While these performance issues are not directly related to the markdown checking implementation, they are important to address for the overall quality of the project.
Suggestions for next steps:
- Consider adding a step in the workflow to run Lighthouse checks automatically, which could help catch performance regressions early.
- Investigate the specific causes of the low performance and PWA scores, and create separate issues to track and address these concerns.
- If applicable, add performance benchmarks to the CI process to ensure that changes don't negatively impact the site's performance.
By addressing these performance concerns alongside the markdown checks, you'll be improving both the content quality and the user experience of the website.
Would you like assistance in creating a workflow step for automated Lighthouse checks or in analyzing the performance issues?
🧰 Tools
🪛 actionlint
82-82: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:3:19: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:4:15: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/if-nodejs-pr-testing.yml (2 hunks)
- scripts/markdown/check-markdown.js (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/if-nodejs-pr-testing.yml
82-82: shellcheck reported issue in this script: SC2129:style:2:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:2:32: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:3:19: Double quote to prevent globbing and word splitting
(shellcheck)
82-82: shellcheck reported issue in this script: SC2086:info:4:15: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 Biome
scripts/markdown/check-markdown.js
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (6)
.github/workflows/if-nodejs-pr-testing.yml (2)
88-102: Approve: Well-implemented PR comment for markdown issuesThis step effectively addresses the PR objective of alerting the team when markdown files have issues. The use of a sticky comment ensures that the information remains visible and up-to-date without cluttering the PR with multiple comments.
The formatting of the comment, including the header and code block for the markdown check results, provides clear and easily readable feedback to the PR author and reviewers.
104-111: Approve: Efficient cleanup of markdown check commentsThis step is a great addition that complements the previous comment step. By deleting the comment when no markdown issues are found, it keeps the PR clean and focused. The implementation correctly uses the output from the markdown check step and the same header to ensure the right comment is managed.
This approach effectively prevents outdated information from lingering on the PR, which aligns well with maintaining a clear and accurate review process.
scripts/markdown/check-markdown.js (4)
10-17: Well-implemented URL validation functionThe
isValidURLfunction is concise, effective, and follows best practices. It properly uses a try-catch block to handle potential errors when creating a URL object, which is an efficient way to validate URLs.
101-135: Well-implemented recursive markdown file checkerThe
checkMarkdownFilesfunction is well-structured and effectively handles directory traversal and file validation. It appropriately uses asynchronous operations for directory listing and file stat checks, while using synchronous file reading for content, which aligns with the project's requirements. The error handling and CI/CD integration throughprocess.exitCodeare also well-implemented.
137-141: Effective main execution setupThe main execution part of the script is concise and well-structured. It correctly resolves the paths for documentation and blog folders, and appropriately calls the
checkMarkdownFilesfunction with the corresponding validation function for each folder.
1-141: Overall, a well-implemented markdown frontmatter validation systemThis script effectively addresses the PR objectives by implementing a comprehensive system for validating the frontmatter in markdown files. The implementation is well-structured, follows good practices, and provides a solid foundation for ensuring the reliability of markdown file rendering.
Key strengths:
- Thorough validation of blog and documentation frontmatter
- Effective recursive file traversal
- Proper error handling and reporting
- Integration with CI/CD through exit code management
The suggested minor improvements for consistency and robustness will further enhance the already strong implementation.
🧰 Tools
🪛 Biome
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
|
This PR is failing because of 2 reasons:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
scripts/markdown/check-markdown.js (5)
10-17: LGTM with a minor suggestionThe
isValidURLfunction provides a simple and effective way to validate URLs. However, for more robust validation, consider adding additional checks, such as ensuring the protocol is http or https, or using a regular expression for stricter validation.For example, you could enhance the function like this:
function isValidURL(str) { try { const url = new URL(str); return url.protocol === "http:" || url.protocol === "https:"; } catch (err) { return false; } }This ensures that only http and https URLs are considered valid.
25-71: LGTM with a suggestion for consistencyThe
validateBlogsfunction provides comprehensive validation for blog post frontmatter. The checks for required attributes, date format, tags, cover, and authors are thorough and well-implemented.For consistency with error handling across the codebase, consider always returning an array, even when there are no errors:
- return errors.length ? errors : null; + return errors;This change would simplify error checking in the calling code, as you'd always be working with an array.
🧰 Tools
🪛 Biome
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
79-93: LGTM with a suggestion for consistencyThe
validateDocsfunction effectively validates the essential attributes for documentation frontmatter. The checks for 'title' and 'weight' are appropriate and well-implemented.For consistency with the suggested change in
validateBlogsand to simplify error handling in the calling code, consider always returning an array:- return errors.length ? errors : null; + return errors;This change would maintain consistency across validation functions and simplify error checking in
checkMarkdownFiles.
101-135: LGTM with a minor suggestion for error loggingThe
checkMarkdownFilesfunction effectively traverses directories and validates markdown files. The use of synchronous file reading within the asynchronous context is intentional and appropriate for handling large markdown files with recursion and loops.Consider enhancing the error logging to include the file path for better traceability:
- console.error('Error reading file stats:', err); + console.error(`Error reading file stats for ${filePath}:`, err);This change would make it easier to identify which specific file caused an error during the validation process.
137-141: LGTM with a suggestion for error handlingThe main execution part of the script correctly sets up the folder paths and calls
checkMarkdownFilesfor both docs and blogs with their respective validation functions.Consider adding error handling for cases where the folder paths might not exist:
const fs = require('fs'); const path = require('path'); const docsFolderPath = path.resolve(__dirname, '../../markdown/docs'); const blogsFolderPath = path.resolve(__dirname, '../../markdown/blog'); [docsFolderPath, blogsFolderPath].forEach(folderPath => { if (!fs.existsSync(folderPath)) { console.error(`Folder not found: ${folderPath}`); process.exitCode = 1; return; } }); if (process.exitCode !== 1) { checkMarkdownFiles(docsFolderPath, validateDocs); checkMarkdownFiles(blogsFolderPath, validateBlogs); }This addition ensures that the script fails gracefully if either of the expected folders is missing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- scripts/markdown/check-markdown.js (1 hunks)
🧰 Additional context used
📓 Learnings (1)
scripts/markdown/check-markdown.js (1)
Learnt from: anshgoyalevil PR: asyncapi/website#3301 File: scripts/markdown/check-markdown.js:31-31 Timestamp: 2024-10-18T17:37:50.602Z Learning: In `scripts/markdown/check-markdown.js`, the `frontmatter` object cannot have a property named `hasOwnProperty`, so using `frontmatter.hasOwnProperty(attr)` is acceptable.
🪛 Biome
scripts/markdown/check-markdown.js
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
markdown/docs/migration/migrating-to-v3.md (4)
Line range hint
18-45: LGTM: Clear explanation of metadata changesThe "Moved metadata" section effectively explains the relocation of
tagsandexternalDocsproperties in v3. The YAML examples clearly illustrate the change.Suggestion for enhancement:
Consider adding a brief explanation of why this change was made (e.g., improved consistency, better organization of metadata). This would help users understand the rationale behind the change.
Line range hint
75-129: LGTM: Comprehensive explanation of major structural changesThe "Operation, channel, and message decoupling" section effectively explains this significant breaking change in v3. It:
- Clearly states that this is the most significant breaking change.
- Explains the new possibilities this change enables (reusing channels, multiple operations per channel).
- Provides context for different messaging systems (Kafka, REST, WebSocket, Socket.IO).
- Includes clear YAML examples that illustrate the structural changes.
Suggestion for enhancement:
Consider adding a brief explanation of the benefits this change brings (e.g., increased flexibility, better representation of complex systems). This would help users understand why this significant change was made.
Line range hint
219-270: LGTM: Clear explanation of message definition changesThe "Messages instead of message" section effectively explains the changes to message definitions in v3. It:
- Clearly describes the shift from using
oneOffor multiple messages to using key-value pairs in the Messages Object.- Provides examples for both single and multiple message scenarios.
- Mentions the removal of the
messageIdproperty and explains how message IDs are now handled.Suggestion for enhancement:
Consider adding a brief explanation of the benefits this change brings (e.g., improved clarity, easier parsing). This would help users understand why this change was made.
Line range hint
272-320: LGTM: Clear explanation of reference unificationThe "Unifying explicit and implicit references" section effectively explains the changes to references in v3. It:
- Clearly states that all references must now be explicit.
- Explains the changes to the Server Object's security property.
- Mentions the relocation of OAuth and OpenID Connect scopes.
- Provides clear YAML examples that illustrate these changes.
Suggestion for enhancement:
Consider adding a brief explanation of why this change was made (e.g., improved consistency, easier parsing or validation). This would help users understand the rationale behind this breaking change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- markdown/docs/migration/index.md (1 hunks)
- markdown/docs/migration/migrating-to-v3.md (1 hunks)
- scripts/markdown/check-markdown.js (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- markdown/docs/migration/index.md
🧰 Additional context used
📓 Learnings (1)
scripts/markdown/check-markdown.js (1)
Learnt from: anshgoyalevil PR: asyncapi/website#3301 File: scripts/markdown/check-markdown.js:31-31 Timestamp: 2024-10-18T17:37:50.602Z Learning: In `scripts/markdown/check-markdown.js`, the `frontmatter` object cannot have a property named `hasOwnProperty`, so using `frontmatter.hasOwnProperty(attr)` is acceptable.
🪛 Biome
scripts/markdown/check-markdown.js
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (15)
scripts/markdown/check-markdown.js (7)
10-17: Well-implemented URL validation functionThe
isValidURLfunction is concise, efficient, and follows best practices for URL validation. The use of a try-catch block to handle potential errors is appropriate here.
25-71: Well-structured blog frontmatter validationThe
validateBlogsfunction provides comprehensive validation for blog frontmatter. It checks all required attributes and performs specific validations for date, tags, cover, and authors.🧰 Tools
🪛 Biome
[error] 31-31: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
79-93: Concise documentation frontmatter validationThe
validateDocsfunction provides focused validation for documentation frontmatter, checking for the presence and correct type of 'title' and 'weight' attributes.
101-140: Well-structured markdown file processingThe
checkMarkdownFilesfunction provides a robust way to recursively process markdown files, apply validation, and report errors. The skipping of the 'docs/reference/specification' folder and setting ofprocess.exitCodeon errors are good practices.
142-146: Effective main execution setupThe script correctly sets up the paths for both documentation and blog folders, and applies the appropriate validation function to each. This ensures comprehensive checking of all relevant markdown files in the project.
70-71: 🛠️ Refactor suggestionConsider returning an empty array instead of null
For consistency with other validation functions, consider returning an empty array instead of
nullwhen there are no errors. This would simplify error handling in the calling code.Apply this diff to improve the return statement:
- return errors.length ? errors : null; + return errors;
92-93: 🛠️ Refactor suggestionConsider returning an empty array instead of null
For consistency with other validation functions, consider returning an empty array instead of
nullwhen there are no errors. This would simplify error handling in the calling code.Apply this diff to improve the return statement:
- return errors.length ? errors : null; + return errors;markdown/docs/migration/migrating-to-v3.md (8)
1-4: LGTM: Frontmatter updateThe addition of the
weight: 2property in the frontmatter is appropriate. This will help control the order of pages in the documentation, indicating that this is an important page in the migration guide.
Line range hint
6-16: LGTM: Clear and informative introductionThe introductory text effectively sets the context for the migration guide. It provides:
- A brief explanation of the migration challenge.
- Information about the AsyncAPI converter tool, including a command for usage.
- A reference to the v3 release notes for more detailed information.
This approach gives users both quick migration options and resources for deeper understanding.
Line range hint
47-73: LGTM: Clear explanation of server URL changesThe "Server URL splitting up" section effectively explains the changes to the Server Object in v3. It:
- Provides context for why the change was made (addressing confusion about URL contents).
- Clearly describes the shift from a single
urlproperty to separatehost,pathname, andprotocolproperties.- Includes clear YAML examples that illustrate the change.
This explanation and the examples will help users understand and implement this breaking change in their AsyncAPI documents.
Line range hint
131-159: LGTM: Clear explanation of channel key and address changesThe "Channel address and channel key" section effectively explains this breaking change in v3. It:
- Clearly describes the shift from using the channel key as the path to using it as an arbitrary unique ID.
- Explains the introduction of the
addressproperty for defining the channel path.- Provides context for why this change was made (enhancing reusability and allowing for scenarios where the same address is used in different contexts).
- Includes clear YAML examples that illustrate the structural changes.
This explanation and the examples will help users understand and implement this breaking change in their AsyncAPI documents.
Line range hint
161-217: LGTM: Comprehensive explanation of operation keyword changesThe "Operation keywords" section effectively explains this important change in v3. It:
- Clearly describes the shift from
publishandsubscribetosendandreceive.- Explains why this change was made (addressing confusion in v2).
- Provides a clear explanation of how the new keywords relate to the application's behavior.
- Includes links to additional resources for users who want more information about the publish/subscribe confusion.
- Provides clear YAML examples that illustrate the changes.
The thorough explanation, examples, and additional resources make this section particularly helpful for users migrating from v2 to v3.
Line range hint
322-376: LGTM: Comprehensive explanation of trait behavior changesThe "New trait behavior" section effectively explains the changes to trait application in v3. It:
- Clearly describes the change in behavior, with properties in the primary objects now taking precedence over those in traits.
- Provides context for why this change was made (preventing unintended overrides).
- Includes clear YAML examples that illustrate the difference in behavior between v2 and v3.
- Mentions the JSON Merge Patch algorithm, providing useful technical context.
- Clearly states the new guideline: "A property on a trait MUST NOT override the same property on the target object".
This detailed explanation and the examples will help users understand and adapt to this breaking change in their AsyncAPI documents.
Line range hint
378-486: LGTM: Clear explanations of various v3 changesThe final sections of the document effectively explain several important changes in AsyncAPI v3:
"Schema format and schemas":
- Clearly explains the introduction of a multi-format schema object.
- Provides context for why this change improves schema reusability.
- Includes clear YAML examples illustrating the change.
"Optional channels":
- Succinctly explains that channels are now entirely optional in v3.
- Provides simple YAML examples showing the difference.
"Restricted parameters object":
- Explains the simplification of the Parameters Object in v3.
- Provides context for why this change was made (focusing on practical use cases).
- Clearly lists the available properties in v3.
- Includes YAML examples illustrating the changes.
These sections provide clear, concise explanations of important changes, helping users understand and implement these changes in their AsyncAPI v3 documents.
Line range hint
1-486: Excellent migration guide: Comprehensive, clear, and well-structuredThis migration guide for AsyncAPI v3 is of high quality and will be extremely valuable for users upgrading from v2. Key strengths include:
- Comprehensive coverage of all major breaking changes.
- Clear explanations that provide context for why changes were made.
- Consistent use of YAML examples to illustrate changes.
- Well-structured format that addresses each change in a logical order.
- Use of custom components (e.g.,
<Asyncapi3MetaComparison>) that likely provide interactive comparisons in the rendered version.Minor suggestions for enhancement:
- Consider adding brief explanations of the benefits for some changes to help users understand the rationale.
- Ensure that the custom comparison components render correctly and enhance the user experience as intended.
Overall, this document serves as an excellent resource for the AsyncAPI community in adopting v3.
|
@asyncapi/bounty_team |
This PR tests the gray matter (front-matter) included in markdown files in the website repository.
Example Comment: anshgoyalevil#17 (comment)
fixes #3215
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
package.jsonto include a script for running markdown checks, streamlining the testing process.