-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: update outdated information on /tools/generator and /tools/cli #3923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis pull request updates several areas of the codebase. In the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/GeneratorInstallation.tsx (1)
1-98: Consider adding context to the code comments.While the code changes are correct, it would be helpful to update the comments on line 25-26 to reflect the new approach. Currently, the comment mentions an output folder flag which is no longer included in the default parameters.
- // By default we will have output folder flag so its set here. + // By default we use the new generator flag, which is needed for HTML Template v3.0.0
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
components/GeneratorInstallation.tsx(1 hunks)package.json(1 hunks)pages/tools/cli.tsx(1 hunks)
🔇 Additional comments (4)
package.json (1)
101-101:Details
✅ Verification successful
Verify compatibility with existing i18n packages.
The addition of
react-i18nextis appropriate for internationalization support. However, ensure it's compatible with the existing i18next (line 79) and next-i18next (line 90) packages already in the dependencies.
🏁 Script executed:
#!/bin/bash # Check compatibility between i18next-related packages npm info i18next@23.12.2 peerDependencies npm info next-i18next@15.3.0 peerDependencies npm info react-i18next@15.4.1 peerDependencies echo "Checking for any compatibility warnings if these packages were installed together:" npm install --package-lock-only --no-package-lock i18next@23.12.2 next-i18next@15.3.0 react-i18next@15.4.1Length of output: 1265
Compatibility Verified: No Action Required
The dependency versions are compatible:
- react-i18next@15.4.1 requires i18next ≥ 23.2.3, which is met by i18next@23.12.2.
- next-i18next@15.3.0 requires i18next ≥ 23.7.13 and compatible React versions, which are satisfied.
- The npm installation simulation did not raise any conflicts related to these packages (aside from minor, unrelated engine warnings).
Please proceed with the current dependency configuration.
pages/tools/cli.tsx (1)
158-158: Great enhancement to CLI description.This update provides a more comprehensive explanation of the CLI's capabilities, accurately reflecting that it can create not just files but complete projects and templates. This aligns with the PR objective to update outdated information.
components/GeneratorInstallation.tsx (2)
26-27: Appropriate update to the generator parameters and specification path.The changes properly address the PR objectives by:
- Simplifying parameters to only include the essential
--use-new-generatorflag- Replacing the shortened URL with a direct link to the current AsyncAPI specification example
These changes ensure compatibility with HTML Template version 3.0.0 as intended.
55-58:Details
✅ Verification successful
Verify template functionality.
The updated specification path and parameters will affect the generated command. Let's ensure the rendered command works correctly with the updated values.
🏁 Script executed:
#!/bin/bash # Verify the AsyncAPI spec URL is accessible curl -s -I "https://raw.githubusercontent.com/asyncapi/spec/refs/heads/master/examples/streetlights-kafka-asyncapi.yml" | head -n 1 # Verify the spec content is valid YAML curl -s "https://raw.githubusercontent.com/asyncapi/spec/refs/heads/master/examples/streetlights-kafka-asyncapi.yml" | grep -q "asyncapi:" && echo "Valid AsyncAPI spec found" || echo "Invalid or missing AsyncAPI spec" # Test if the HTML template can be found on npm npm view @asyncapi/html-template@3.0.0 versionLength of output: 397
Template functionality verified:
The verification steps confirm that the AsyncAPI spec URL is accessible (HTTP/2 200), returns valid YAML (contains the "asyncapi:" keyword), and the HTML template is available on npm at version 3.0.0. This ensures that the generated command incomponents/GeneratorInstallation.tsxworks as expected with the updated specification path and parameters.No further changes are required.
|
Hi @derberg @akshatnema @anshgoyalevil @Mayaleeeee @asyncapi-bot-eve, |
|
Hi @derberg @akshatnema @anshgoyalevil @Mayaleeeee, PR #3923 is failing due to 4 failing checks. |
Fixed: issue #3721
1st Issue:
Description:
This PR updates the incorrect installation example in the AsyncAPI documentation for the HTML template.
Changes Made:
Updated the specPath for HTML Template v3.0.0:
Previous specPath: Path (used for AsyncAPI v2.0.0)
New specPath: Path
This ensures compatibility with @asyncapi/html-template@3.0.0.
2nd Issue:
Description: Updated Description as per PR #1649
Changes Made: Changed the description text.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Chores