-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: update doc links to point at new site #20754
Conversation
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, but i added @sanderson to double check the links
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.
One minor suggestion. Feel free to ignore. LGTM.
@@ -61,14 +61,14 @@ export class SelectCollectorsStep extends PureComponent<Props> { | |||
<h5 className="wizard-step--sub-title"> | |||
Looking for other things to monitor? Check out our 200+ other | |||
<a | |||
href="https://v2.docs.influxdata.com/v2.0/reference/telegraf-plugins/#input-plugins" | |||
href="https://docs.influxdata.com/telegraf/v1.17/plugins/#input-plugins" |
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.
Not a show-stopper, but you may update this to https://docs.influxdata.com/telegraf/latest/plugins/#input-plugins
just so it doesn't need to be updated for each Telegraf release.
href="https://docs.influxdata.com/telegraf/v1.17/plugins/#input-plugins" | |
href="https://docs.influxdata.com/telegraf/latest/plugins/#input-plugins" |
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.
Would it make sense to do the same for all the links that start with docs.influxdata/com/influxdb/v2.0
? (To make them docs.influxdata/com/influxdb/latest
instead)
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.
This is where it gets tricky. The InfluxDB docs are versioned per minor version of InfluxDB, so it makes sense to leave these as-is and then update them to v2.1 whenever 2.1 rolls out. That way the links in the UI point to the specific version of InfluxDB the user is running. The tricky part is that this same code is used (afaik) in the Cloud UI, so links in cloud are actually pointing to OSS docs. So in short... I don't know 🤷🏻♂️
In an ideal world, we'd be able to have these links be version/context specific.
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.
What I'll do is:
- Update the links on
master
to uselatest
- When I backport this to the
2.0
branch, keep the links pointing at2.0
Closes #20673