-
Notifications
You must be signed in to change notification settings - Fork 282
[editorial] Update database redirect rules #3140
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
base: main
Are you sure you want to change the base?
[editorial] Update database redirect rules #3140
Conversation
Signed-off-by: Vitor Vasconcellos <[email protected]>
Signed-off-by: Vitor Vasconcellos <[email protected]>
…cs/README.md` Signed-off-by: Vitor Vasconcellos <[email protected]>
…cs/README.md` Signed-off-by: Vitor Vasconcellos <[email protected]>
Signed-off-by: Vitor Vasconcellos <[email protected]>
chalin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks @vitorvasc!
Did you try adding the redirects rule to the db/README.md?
chalin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM / is functional as is. I'd prefer having the db redirects more tightly scoped if that'll work. (Approving now in case I can't get back to it later.)
|
@chalin I did try at first, but I didn't really test it integrating with open-telemetry/opentelemetry.io#8311. I was trying to test everything locally, but none of the redirect rules were working correctly (I'll follow-up about that in the #otel-comms-approvers thread we've been discussing it). My thinking was that if we added the rule to |
👍🏻 Yeah, Netlify CLI doesn't always play nice when run locally.
The rules have special-case handling to avoid that. Either you'll need to use |
39a3859 to
60f849e
Compare
|
@vitorvasc - even if you can't test using Netlify CLI, you can always inspect the redirects file that gets generated. |
Signed-off-by: Vitor Vasconcellos <[email protected]>
💡 💡 ! Okay, just tested it by checking the generated |
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
…o docs/README.md Signed-off-by: Vitor Vasconcellos <[email protected]>
chalin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @vitorvasc for trying out the alternatives.
LGTM
trask
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #3138. Also referenced by open-telemetry/opentelemetry.io#8311.
Changes
docs/README.md.You can test the updates and the redirect rules here: https://deploy-preview-8311--opentelemetry.netlify.app.
When navigating to: https://deploy-preview-8311--opentelemetry.netlify.app/docs/specs/semconv/database/, it now correctly redirects to the new path: https://deploy-preview-8311--opentelemetry.netlify.app/docs/specs/semconv/db/.