-
-
Notifications
You must be signed in to change notification settings - Fork 724
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: refactor the 'Edit this page on GitHub' workflow (#3221) #3546
Conversation
WalkthroughThe pull request introduces changes to improve the GitHub edit link functionality. In the Changes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 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. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3546--asyncapi-website.netlify.app/ |
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
Plan: Pro
📒 Files selected for processing (2)
components/layout/DocsLayout.tsx
(1 hunks)config/edit-page-config.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- config/edit-page-config.json
🧰 Additional context used
🪛 GitHub Actions: PR testing - if Node project
components/layout/DocsLayout.tsx
[error] 63-63: Prettier formatting error: Attributes should be on separate lines for better readability in target='_blank' rel='noopener noreferrer' href element
🔇 Additional comments (1)
components/layout/DocsLayout.tsx (1)
63-63
: Validate slicing logic for the post ID
Removing the last character fromlast
will break the link if there is no trailing slash or extra character at the end. Consider checking whether the post ID actually ends with a slash (or any undesired character) before slicing.You can run a quick search to confirm that
post.id
orlast
always has an extra trailing slash:✅ Verification successful
Let me gather more context about how
post.id
is structured and used.
Let me check how the
post.id
is structured by looking at where it's defined and any test files.
Let me try a different approach to understand the post structure and usage.
Let me check the post type definition and any example usage.
Based on the code analysis, I can now provide a final response:
The slicing logic is safe and intentional
The code inDocsLayout.tsx
shows thatlast
is derived frompost.id
usingsubstring(post.id.lastIndexOf('/') + 1)
, which means it already contains just the final segment after the last slash. Theslice(0, -1)
operation is intentionally removing the trailing character (likely a file extension) to construct the correct GitHub edit URL based on theeditOptions
configuration.Additionally, there's a safety check with
if (isHrefToFile) last = ''
that prevents any potential issues when the target is a direct file path.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential references or usage of 'post.id' or 'last' in the codebase to confirm trailing slash patterns: rg 'post\.id|last\s*=' -A 3Length of output: 619
Script:
#!/bin/bash # Search for assignments or declarations of post.id to understand its structure ast-grep --pattern 'post.id = $_' # Also search for any direct usage of post.id rg "post\.id" -B 2 -A 2 # Let's also check the structure of the entire component cat components/layout/DocsLayout.tsxLength of output: 9292
Script:
#!/bin/bash # Search for IPost interface definition to understand post.id structure ast-grep --pattern 'interface IPost { $$$ }' # Look for any test files that might show example post.id values rg -g '*.test.*' -g '*.spec.*' "post\.id" # Check for any mock data files that might contain example posts fd -e json -e ts mockLength of output: 116
Script:
#!/bin/bash # Look for the IPost type definition rg "interface IPost" -A 15 # Check the getAllPosts implementation since it's used in the component rg "getAllPosts" -A 5 # Look for any files that might contain post data structure fd -e ts -e tsx -e json . | rg -i "post|docs"Length of output: 5326
Script:
#!/bin/bash # Check the post type definition cat types/post.ts # Look for any example usage in the codebase rg -g '*.json' '"id":' -B 1 -A 3 # Check the edit options configuration cat config/edit-page-config.jsonLength of output: 20894
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 63-63: Prettier formatting error: Attributes should be on separate lines for better readability in target='_blank' rel='noopener noreferrer' href element
components/layout/DocsLayout.tsx
Outdated
@@ -60,7 +60,7 @@ function generateEditLink(post: IPost) { | |||
if (isHrefToFile) last = ''; | |||
|
|||
return ( | |||
<a target='_blank' rel='noopener noreferrer' href={`${target?.href}/${last}`} className='ml-1 underline'> | |||
<a target='_blank' rel='noopener noreferrer' href={`${target?.href}/${last.slice(0, -1)}`} className='ml-1 underline'> |
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.
Resolve Prettier formatting error
The pipeline reports a formatting error for this anchor element. According to the style guidelines, each attribute should be placed on its own line.
Below is a proposed fix:
- <a target='_blank' rel='noopener noreferrer' href={`${target?.href}/${last.slice(0, -1)}`} className='ml-1 underline'>
+ <a
+ target='_blank'
+ rel='noopener noreferrer'
+ href={`${target?.href}/${last.slice(0, -1)}`}
+ className='ml-1 underline'
+ >
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<a target='_blank' rel='noopener noreferrer' href={`${target?.href}/${last.slice(0, -1)}`} className='ml-1 underline'> | |
<a | |
target='_blank' | |
rel='noopener noreferrer' | |
href={`${target?.href}/${last.slice(0, -1)}`} | |
className='ml-1 underline' | |
> |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 63-63: Prettier formatting error: Attributes should be on separate lines for better readability in target='_blank' rel='noopener noreferrer' href element
Semantic PR Title Check
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3546 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 668 668
=========================================
Hits 668 668 ☔ View full report in Codecov by Sentry. |
The GitHub issue participates in the AsyncAPI Bounty Program and is the responsibility of the assigned user. You are welcome to choose another GitHub issue for contribution that does not have the |
Description
This Pull Request addresses the broken or incorrect “Edit this page on GitHub” links by modifying two key files:
config/edit-page-config.json
tree/master
references toblob/master
./tools/generator
,/tools/cli
, and other sections:components/layout/DocsLayout.tsx
last.slice(0, -1)
:These changes ensure the “Edit this page” link directs contributors to the correct markdown files on GitHub.
Related Issue
Closes #3221.
Additional Notes
Recording.2025-01-06.001601.1.mp4
Checklist
@sambhavgupta0705 @anshgoyalevil @akshatnema
Summary by CodeRabbit