Skip to content
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

[DX-1696] Check for broken external urls #5498

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

yurisasuke
Copy link
Member

@yurisasuke yurisasuke commented Oct 2, 2024

User description

We only check for internal broken links. We should also make sure that we also check for external ones.
DX-1696

This solution has been closed since:

  1. It is treating some internal links as external links. (especially those links without http in them(relative links) )

PR Type

enhancement, configuration changes


Description

  • Enabled checking for broken external URLs in the .htmltest.yml configuration file.
  • Excluded specific URLs from the external link checks to prevent false positives.
  • Removed an unnecessary preconnect link to Algolia in the footer scripts.

Changes walkthrough 📝

Relevant files
Enhancement
footer_scripts.html
Remove preconnect link to Algolia in footer scripts           

tyk-docs/themes/tykio/layouts/partials/footer_scripts.html

  • Removed a preconnect link to Algolia.
+0/-1     
Configuration changes
.htmltest.yml
Enable external link checking and exclude specific URLs   

.htmltest.yml

  • Enabled external link checking.
  • Excluded specific GitHub and support URLs from checks.
  • +3/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    github-actions bot commented Oct 2, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit 7871fe9)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Configuration Change
    The change to enable external link checking (CheckExternal: true) could potentially increase the build times and should be tested to ensure it does not affect the deployment pipeline adversely.

    Removed Code
    The removal of the preconnect link to Algolia might affect the performance of the site's search functionality. This should be verified to ensure that there are no negative impacts on user experience.

    Copy link
    Contributor

    github-actions bot commented Oct 2, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Improve URL matching accuracy in the ignore list

    Add a trailing dollar sign to the regex for ignoring URLs to ensure it only matches
    the exact URLs specified, avoiding potential mismatches with similar URLs.

    .htmltest.yml [10-11]

    -- "^https://github.com/TykTechnologies/tyk-docs"
    -- "^https://support.tyk.io"
    +- "^https://github.com/TykTechnologies/tyk-docs$"
    +- "^https://support.tyk.io$"
    Suggestion importance[1-10]: 8

    Why: Adding a trailing dollar sign to the regex ensures that only the exact URLs are matched, preventing potential mismatches with similar URLs. This enhances the accuracy of the ignore list, making it a valuable improvement.

    8

    Copy link

    netlify bot commented Oct 2, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit bc1618e
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/6763a645a437340008d9d427
    😎 Deploy Preview https://deploy-preview-5498--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @yurisasuke
    Copy link
    Member Author

    closing as i look for a better solution.Current solution has so many unnecessary errors.

    Copy link
    Contributor

    github-actions bot commented Oct 7, 2024

    Persistent review updated to latest commit caef11c

    Copy link
    Contributor

    github-actions bot commented Oct 7, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Improve URL matching accuracy in the ignore list

    Add a trailing dollar sign to the regex for ignoring URLs to ensure it only matches
    the exact URLs specified, avoiding potential mismatches with similar URLs.

    .htmltest.yml [10-11]

    -- "^https://github.com/TykTechnologies/tyk-docs"
    -- "^https://support.tyk.io"
    +- "^https://github.com/TykTechnologies/tyk-docs$"
    +- "^https://support.tyk.io$"
    Suggestion importance[1-10]: 8

    Why: Adding a trailing dollar sign to the regex ensures that only the exact URLs are matched, preventing potential mismatches with similar URLs. This enhances the accuracy of the ignore list, making it a valuable improvement.

    8

    @yurisasuke yurisasuke closed this Oct 8, 2024
    @yurisasuke yurisasuke reopened this Nov 26, 2024
    Copy link
    Contributor

    Persistent review updated to latest commit 7871fe9

    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant