-
-
Notifications
You must be signed in to change notification settings - Fork 697
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
fix: specification explorer layout issue by bumping schyma #3442
fix: specification explorer layout issue by bumping schyma #3442
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3442 +/- ##
=======================================
Coverage 86.59% 86.59%
=======================================
Files 21 21
Lines 664 664
=======================================
Hits 575 575
Misses 89 89 ☔ View full report in Codecov by Sentry. |
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
(2 hunks)
🔇 Additional comments (1)
package.json (1)
113-113
: Verify the schyma version bump resolves the layout issue.
The version bump of schyma to ^1.0.5 aligns with the PR objective to fix the specification explorer layout issue.
Let's verify this change:
@@ -151,13 +151,13 @@ | |||
"eslint-plugin-storybook": "^0.8.0", | |||
"eslint-plugin-tailwindcss": "^3.14.2", | |||
"eslint-plugin-unused-imports": "^3.1.0", | |||
"fast-xml-parser": "^4.5.0", |
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.
💡 Codebase verification
The fast-xml-parser addition is unrelated to the specification explorer layout fix
The fast-xml-parser package is being used in the RSS feed testing functionality (tests/build-rss.test.js) to validate XML output from the RSS feed generator. This change appears to have been accidentally included in this PR as it's unrelated to the specification explorer layout fix. It should be moved to a separate PR focused on RSS feed functionality.
🔗 Analysis chain
Clarify the addition of fast-xml-parser.
The addition of fast-xml-parser to devDependencies seems unrelated to the specification explorer layout fix. Please clarify if this change is intentional and necessary for this PR.
Let's check if this dependency is related to schyma or the layout fix:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if fast-xml-parser is a dependency of schyma or used in related files
# Check if fast-xml-parser is a dependency of schyma
curl -s https://registry.npmjs.org/schyma/1.0.5 | jq -r '.dependencies["fast-xml-parser"] // empty'
# Search for fast-xml-parser usage in the codebase
rg -l "fast-xml-parser" --type js --type ts
Length of output: 169
Script:
#!/bin/bash
# Let's check the content of the test file to understand the usage
cat tests/build-rss.test.js
# Let's also check for any recent changes related to RSS or XML parsing
git diff HEAD~1 tests/build-rss.test.js
# Check if there are any other XML-related files
rg -l "\.xml" --type js --type ts
Length of output: 6117
Script:
#!/bin/bash
# Let's check the recent changes to see if RSS functionality was added
git diff HEAD~1 scripts/build-rss.js
# Check the PR description or commit messages for context
git log -1 --pretty=%B
# Let's also check if there are any layout-related changes
rg -l "layout" --type js --type ts
Length of output: 1068
Script:
#!/bin/bash
# Let's check the content of the build-rss.js script to see if it's a new addition
cat scripts/build-rss.js
# Let's also check for any changes in the package.json related to RSS or XML
git diff HEAD~1 package.json
Length of output: 4290
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3442--asyncapi-website.netlify.app/ |
@catosaurusrex2003 I don't see any errors in the current website. |
@akshatnema If you're still unable to reproduce the issue, could you please let me know what browser you're using? 👀 |
…x2003/website into schyma-version-bump
…into schyma-version-bump
/update |
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.
@catosaurusrex2003 I reproduced the error in the current website and checked it in your PR also.
It is working fine in your PR
Thanks!!
LGTM 🚀
hey @anshgoyalevil @sambhavgupta0705 @akshatnema lets pause the merging of this PR for the time being. I have raised another PR in schyma. lets merge this PR after that one gets merged cuz the version will bump again. |
…x2003/website into schyma-version-bump
…into schyma-version-bump
@akshatnema @sambhavgupta0705 @anshgoyalevil we can go forward with the merging now |
/rtm |
The issue was fixed in this PR AceTheCreator/schyma#5
Related issue(s)
#3334
Summary by CodeRabbit
schyma
package version to improve performance.fast-xml-parser
to enhance XML parsing capabilities.devDependencies
by removing the previous entry forfast-xml-parser
.