Workflow changes to add automated link checking#428
Conversation
Co-authored-by: TimHess <3947063+TimHess@users.noreply.github.com>
TimHess
changed the title
…and-stage.yml Co-authored-by: TimHess <3947063+TimHess@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Preview link: https://www-steeltoe-pr-428.azurewebsites.net
|
TimHess
changed the title
…cfx files - ignore razor routes from static file checking - alter root-dir to avoid errors with asset links
- drop changes to build-and-stage - replace .ps1 with equivalent of check-links.yml
Co-authored-by: Cursor <cursoragent@cursor.com> Made-with: Cursor
Made-with: Cursor
.github/workflows/check-links.yml
Outdated
| --retry-wait-time 15 | ||
| --timeout 30 | ||
| '**/*.md' | ||
| fail: true |
There was a problem hiding this comment.
Why does the step need to fail when broken links are found? Without it, the ifs in subsequent steps can be removed. And then we can distinguish between broken links and unexpected errors.
There was a problem hiding this comment.
I guess it doesn't need to fail, but without the job failing it's a lot easier to skip over it... We don't have it as a required check, so it's not like this ever prevents a merge
There was a problem hiding this comment.
We have the summary for that, and we don't want to fail on broken links. But we should be able to distinguish between broken links (which are in the report) vs errors like "python error xxx", "commit sha not found", etc.
There was a problem hiding this comment.
despite removing fail:true the job still shows failure, so this flag doesn't seem to matter either way
There was a problem hiding this comment.
It defaults to true. Have you tried setting it to false?
.github/workflows/check-links.yml
Outdated
| uses: lycheeverse/lychee-action@v2 | ||
| with: | ||
| args: | | ||
| --verbose |
There was a problem hiding this comment.
Why verbose output? Should we pass --quiet instead?
There was a problem hiding this comment.
Verbose output for at least this stage of the work to see which links are being checked. I suppose we could remove it now, we could remove it in the PR that fixes links, or we could keep it so we always have a way to see what it actually checked. We also could let it use the default and not pass either of those flags
There was a problem hiding this comment.
Unresolving because I put it back (in script and workflow) so we don't lose the details on redirects if we remove it... they aren't in the logs or summary otherwise. I think we should keep it at least through 424
There was a problem hiding this comment.
I'm fine with enabling verbose logging in the workflow and script if that helps understand why something was reported. But can we hide the redirects from the PR comment? They seem just noise to me; after all, the links are working, so there's no need to "fix" them.
There was a problem hiding this comment.
After the next CI run in 424, assuming the results look as clear as we want them to, I'll remove the verbosity setting in that PR.
There was a problem hiding this comment.
Worth noting... the redirects have disappeared from the comment in this PR because the most recent workflow run used the cache from the previous run. Lychee only caches the final result, which does not track the redirects.
The GHA cache should stick around for 7 days, but lychee has a default max-cache-age of 1 day, so any older GHA cache hit will not be used by lychee.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Bart Koelman <104792814+bart-vmware@users.noreply.github.com>
- only use --no-progress in CI - use more defaults - change settings per-host instead of globally - check for installed lychee first
This comment was marked as resolved.
This comment was marked as resolved.
--verbose is needed to see redirect details - locally: - set user-agent locally to avoid 403 on mysql links (doesn't seem to affect CI) - ignore build/docfx-net10-binaries (like playwright readme) - longer host-request-interval and retry-wait-time
.github/workflows/check-links.yml
Outdated
| --scheme http | ||
| --scheme https |
There was a problem hiding this comment.
In the local script, these are quoted values.
- since redirect fixes are already in place in other PR, change verbosity now
- also include a paste-able snippet to bust the cache for a given PR
Use lychee to add a broken external link checker workflow. Also addresses all workflow-related review feedback from PR #424.
check-links.yml(new)pull_request(paths:**.md,docs/**, and the workflow file itself) andworkflow_dispatchlocalhost,fortuneservice,*.internal,consul-register-example,docker://; paths.git,docs/api--max-concurrency 5,--retry-wait-time 15,--max-retries 6,--timeout 30if: ${{ !cancelled() }}; lychee's built-in# Summaryheading replaced with# Link checking resultsname: Git checkout,persist-credentials: false💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.