Skip to content

add a couple notes about redirect particularities.#2635

Open
ElliotFriend wants to merge 1 commit into
mainfrom
chore/note-redirect-weirdness
Open

add a couple notes about redirect particularities.#2635
ElliotFriend wants to merge 1 commit into
mainfrom
chore/note-redirect-weirdness

Conversation

@ElliotFriend

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings July 18, 2026 14:25

Copilot AI left a comment

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.

Pull request overview

Adds guidance for writing and reviewing NGINX redirects.

Changes:

  • Adds inline redirect syntax warnings.
  • Expands contributor guidance for regex-based redirects.

Recommendation: NEEDS-CHANGES — align the conflicting curly-brace guidance and clarify the NGINX parser constraint.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
nginx/includes/redirects.conf Adds redirect-authoring cautions.
CONTRIBUTING.md Documents regex quoting and review requirements.

Comment on lines +1 to +2
## nginx redirects file. write carefully, syntax must be precise. don't use
## curly braces "{}" in these. it's a headache.
Comment thread CONTRIBUTING.md

To the extent possible, anything that changes a page's URL needs redirect coverage (`routes.txt` participates in CI route checking; server-side redirects live in `nginx/`). If your PR moves or renames pages, call it out explicitly in the description.

Redirects written for nginx can be tricky. The syntax MUST be precise, and regular expressions don't always behave like they do elsewhere (e.g., regexes with curly braces `{}` must be fully contained by quotes `"`). If a URL rewrite is introduced using a regular expression, a close test and review from maintainers is required.
@stellar-jenkins-ci

Copy link
Copy Markdown

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.

2 participants