Skip to content

test: Example PR that fails the redirect check (do not merge) - #1965

Draft
marcleblanc2 wants to merge 4 commits into
mainfrom
marc/example-check-redirects
Draft

marcleblanc2 wants to merge 4 commits into
mainfrom
marc/example-check-redirects

Conversation

@marcleblanc2

@marcleblanc2 marcleblanc2 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Example for the docs-maintainer announcement: one PR, one failing check. Do not merge.

Adds four entries to src/data/redirects.ts, each breaking one rule: the source shadows a page that exists, a /docs prefix, a redirect whose destination is another redirect, and a destination page that does not exist. The Check redirects workflow should comment a report and suggest fixes for the two it can fix.

Built on main plus #1935 (marc/check-redirects, rebased) because the check is not on main yet, so the diff also shows the check's own files. The last commit is the example; once #1935 merges, only that commit remains.

Siblings: #1963 spell check and #1964 link check. Replaces the all-in-one #1948 for the announcement.

@vercel

vercel Bot commented Sep 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sourcegraph-docs Ready Ready Preview Sep 15, 2026 9:08pm UTC

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

❌ This PR breaks 4 redirect(s)

Redirects are used so inbound traffic from external sources (links inside old versions of our product, bookmarks, search results, etc.) to old doc pages still reaches a relevant page.

A correct entry maps the old page path, exactly as the browser requests it, straight to a page that exists today, with an optional #heading that exists on the destination page:

{
	source: '/old/section/page',
	destination: '/new/section/page#heading-slug'
},

Each section below explains how to fix the entries listed under it.

Do not use redirects for broken internal links, internal links must be fixed properly to tame the tech debt snowball no one wants to deal with; the "Check links" PR check comment lists the links this PR broke, if any.

src/data/redirects.ts

Source overshadows a docs page that exists

Redirects take precedence over pages, so visitors to that page's URL are redirected away from it. Update or remove the redirect or the page to remove the conflict.

  • line 5
    source: '/code-ownership',
    destination: '/code-ownership/codeowners-format'

Source or destination starts with /docs

Write paths without the /docs prefix. The site removes /docs from the requested URL before matching sources, and adds it back in front of the destination, so a /docs/... source never matches and a /docs/... destination lands on /docs/docs/....

  • line 9
    source: '/docs/ownership',
    destination: '/code-ownership'
    Fix: change the source to /ownership

Destination is another redirect

Chained redirects cost the customer's browser a round trip, slow down their page load time, and frustrate them. They also make the redirects file impossible to maintain, and make it too easy to create redirect loops. Change the rule's destination to the final destination.

  • line 13
    source: '/ownership/tls',
    destination: '/admin/tls_ssl'
    Fix: change the destination to /self-hosted/http-https-configuration

Destination page does not exist

Set the redirect destination to the page that replaced it, or remove the rule if there is no replacement page; visitors then get our fancy 404 page, with links they can click to find where they're trying to go, and the search bar.

  • line 17
    source: '/ownership/guide',
    destination: '/code-ownership/guide'

Reproduce locally with node dev/check-redirects.mjs

marcleblanc2 and others added 3 commits September 15, 2026 15:04
dev/check-redirects.mjs checks every entry in src/data/redirects.ts:
source shadows a page, source has a #fragment, duplicate source,
/docs prefix, chained redirect, missing destination page or heading.
The workflow compares against the merge base, so only redirects a PR
breaks are reported, grouped by problem with the fix explained under
each heading, and posts one suggested change per fixable entry the PR
added (deleted again once the finding is gone).

Not part of `npm run check`: main has hundreds of pre-existing
findings.

Squash of the check-redirects branch rebased onto main; the check-links
commits it carried are already on main.

Amp-Thread-ID: https://ampcode.com/threads/T-01a08fee-74b4-76dc-aaf9-d1245d68fdc9
Co-authored-by: Amp <amp@ampcode.com>
@marcleblanc2
marcleblanc2 force-pushed the marc/example-check-redirects branch from 3ff054c to 651bccf Compare September 15, 2026 21:04
@marcleblanc2
marcleblanc2 changed the base branch from marc/check-redirects to main September 15, 2026 21:04
Comment thread src/data/redirects.ts
Comment on lines +8 to +11
{
source: '/docs/ownership',
destination: '/code-ownership'
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem: Source or destination starts with /docs
Fix: change the source to /ownership

Suggested change
{
source: '/docs/ownership',
destination: '/code-ownership'
},
{
source: '/ownership',
destination: '/code-ownership'
},

Comment thread src/data/redirects.ts
Comment on lines +12 to +15
{
source: '/ownership/tls',
destination: '/admin/tls_ssl'
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem: Destination is another redirect
Fix: change the destination to /self-hosted/http-https-configuration

Suggested change
{
source: '/ownership/tls',
destination: '/admin/tls_ssl'
},
{
source: '/ownership/tls',
destination: '/self-hosted/http-https-configuration'
},

Adds four redirects, each breaking one rule: shadows an existing page, /docs prefix, redirect to a redirect, destination page missing.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0a6dc-7fb8-766d-8e8d-c2fda22ee1ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant