- Notifications
You must be signed in to change notification settings - Fork927
fix: make default support links respect --docs-url#14176
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix: make default support links respect --docs-url#14176
Uh oh!
There was an error while loading.Please reload this page.
Conversation
alwaysmeticulousbot commentedAug 5, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Meticulous spotted zero visual differences across 1324 screens tested:view results. Expected differences?Click here. Last updated for commit a756f3a. This comment will update as new commits are pushed. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
coderd/appearance/appearance.go Outdated
| { | ||
| func DefaultSupportLinks(docsURL string) []codersdk.LinkConfig { | ||
| if docsURL == "" { | ||
| docsURL = "https://coder.com/docs/{CODER_VERSION}" |
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.
You could usebuildinfo.Version() instead so the frontend doesn't have to replace this.
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.
Yeah I was taking a "do as the Romans do" approach and doing it the way the build info was done, but I agree with your comment below that we have this info on the backend so we might as well do it here.
Let me ping someone for context on why it's set up this way before falling into the classic "refactor and then realize why they did it the original way" trap and get back to you haha.
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.
Looks like we can just have the backend do this :)
https://codercom.slack.com/archives/CJURPL8DN/p1723126779529959
coderd/appearance/appearance.go Outdated
| defaultSupportLinks := []codersdk.LinkConfig{ | ||
| { | ||
| Name: "Report a bug", | ||
| Target: "https://github.com/coder/coder/issues/new?labels=needs+grooming&body={CODER_BUILD_INFO}", |
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.
I know you didn't introduce this, but I think we should be doing this replacement on the backend, not the frontend. We already should have this info anyways.
@kylecarbs do you mind re-reviewing when you get a chance? |
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 and clean!
76722a7 intomainUh oh!
There was an error while loading.Please reload this page.
The default support links previously included a link to Coder's hosted docs, but did not respect the
--docs-urldeployment value. This meant that on the User Dropdown on the site, there was a link to docs that didn't respect the user's set--docs-url(which is not exactly a bug, but definitely confusing).This PR updates the default support links to respect the
--docs-urldeployment value. If the value is not set, it will use the hosted coder docs url as before.