Fix edit link generation and reduce 404 errors#5318
Open
ksaini-web wants to merge 1 commit into
Open
Conversation
ksaini-web
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
Contributor
|
We require all PRs to follow Conventional Commits specification. |
Contributor
📝 WalkthroughWalkthroughThe changes address broken edit link generation by adjusting configuration URLs and refactoring the edit link logic to use full relative paths with cross-platform path normalization and prefix removal for specification references. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Contributor
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5318--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
config/edit-page-config.json (1)
2-5: Minor JSON formatting inconsistency.The first entry (lines 2-5) lacks the 2-space indentation that all other entries in the array have. This doesn't affect functionality but creates inconsistent formatting.
🔧 Suggested fix for consistent indentation
[ -{ - "value": "reference/bindings/", - "href": "https://github.com/asyncapi/bindings/blob/master" -}, + { + "value": "reference/bindings/", + "href": "https://github.com/asyncapi/bindings/blob/master" + }, { "value": "/tools/generator",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/edit-page-config.json` around lines 2 - 5, The JSON entry with "value": "reference/bindings/" and "href": "https://github.com/asyncapi/bindings/blob/master" has inconsistent indentation compared to the other array entries; reformat that object to use the same 2-space indentation as the rest of entries in edit-page-config.json so it aligns visually with surrounding items (locate the object by the "value": "reference/bindings/" key to make the change).scripts/markdown/check-edit-links.ts (2)
154-172: Inconsistent indentation within the function.The code blocks from lines 154-172 have inconsistent indentation compared to lines 143-152. This affects readability and maintainability.
🔧 Suggested fix for consistent indentation
-// Safety fallback: if extraction fails, use filename only -if (!relativePath) { - return `${target.href}/${path.basename(filePath)}`; -} - -// Some external repos do not keep the same docs-side folder structure, -// so remove those prefixes before building the final edit link. -let repoRelativePath = relativePath; - -if (target.href.includes('asyncapi/bindings')) { - repoRelativePath = repoRelativePath.replace('reference/bindings/', ''); -} - -if (target.href.includes('asyncapi/spec')) { - repoRelativePath = repoRelativePath.replace('reference/specification/', ''); -} - -// Construct correct GitHub edit link -return `${target.href}/${repoRelativePath}`; + // Safety fallback: if extraction fails, use filename only + if (!relativePath) { + return `${target.href}/${path.basename(filePath)}`; + } + + // Some external repos do not keep the same docs-side folder structure, + // so remove those prefixes before building the final edit link. + let repoRelativePath = relativePath; + + if (target.href.includes('asyncapi/bindings')) { + repoRelativePath = repoRelativePath.replace('reference/bindings/', ''); + } + + if (target.href.includes('asyncapi/spec')) { + repoRelativePath = repoRelativePath.replace('reference/specification/', ''); + } + + // Construct correct GitHub edit link + return `${target.href}/${repoRelativePath}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/markdown/check-edit-links.ts` around lines 154 - 172, The block handling the safety fallback and repo-specific path fixes in check-edit-links.ts (references: variables relativePath, repoRelativePath, target.href, filePath and the final return) has inconsistent indentation compared to the earlier logic; reformat that entire section so its indentation style matches the surrounding lines (align the if/return blocks and inner repoRelativePath.replace calls with the earlier code style), preserving the same statements and behavior but making bracketed blocks and return lines consistently indented for readability.
174-181: Minor code style inconsistencies.There are a few formatting issues:
- Extra blank lines inside the
ifblock (lines 174-175)- Missing semicolon on
return null(line 178)- Extra blank lines after the function closing brace
🔧 Suggested cleanup
// Construct correct GitHub edit link return `${target.href}/${repoRelativePath}`; - - - } - // If no matching config found, return null -return null -} - - + // If no matching config found, return null + return null; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/markdown/check-edit-links.ts` around lines 174 - 181, Fix the minor formatting issues in the function at the end of scripts/markdown/check-edit-links.ts that returns a matching config or null: remove the extra blank lines inside the if block, add a missing semicolon to the `return null` statement, and delete the stray blank lines after the function's closing brace so the function ends cleanly. Locate the function that performs the matching and ends with `return null` and apply these style corrections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/edit-page-config.json`:
- Around line 2-5: The JSON entry with "value": "reference/bindings/" and
"href": "https://github.com/asyncapi/bindings/blob/master" has inconsistent
indentation compared to the other array entries; reformat that object to use the
same 2-space indentation as the rest of entries in edit-page-config.json so it
aligns visually with surrounding items (locate the object by the "value":
"reference/bindings/" key to make the change).
In `@scripts/markdown/check-edit-links.ts`:
- Around line 154-172: The block handling the safety fallback and repo-specific
path fixes in check-edit-links.ts (references: variables relativePath,
repoRelativePath, target.href, filePath and the final return) has inconsistent
indentation compared to the earlier logic; reformat that entire section so its
indentation style matches the surrounding lines (align the if/return blocks and
inner repoRelativePath.replace calls with the earlier code style), preserving
the same statements and behavior but making bracketed blocks and return lines
consistently indented for readability.
- Around line 174-181: Fix the minor formatting issues in the function at the
end of scripts/markdown/check-edit-links.ts that returns a matching config or
null: remove the extra blank lines inside the if block, add a missing semicolon
to the `return null` statement, and delete the stray blank lines after the
function's closing brace so the function ends cleanly. Locate the function that
performs the matching and ends with `return null` and apply these style
corrections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f36517ac-4fae-4a3d-8013-36625c931c69
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
config/edit-page-config.jsonscripts/markdown/check-edit-links.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR improves how edit links are generated in
check-edit-links.ts.Earlier, the script was using only the file name to build edit links. Because of this, many links were incorrect and resulted in 404 errors.
What I changed
Result
After these changes, many incorrect edit links are fixed and 404 errors are reduced.
Note
Some links are still showing as invalid because of differences in external repository structures (for example, bindings repo uses folder-based README files). These are not caused by this script.
Resolves #5151
Summary by CodeRabbit