-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Docs: Update Loki GitHub URLs #9080
Docs: Update Loki GitHub URLs #9080
Conversation
* use "main" instead of "master" branch * use a stable version in case lines are referenced * fix outdated URLs
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.
[docs team] Thank you for submitting this PR with much needed updates.
|
||
* Update the code to fix a bug or add a new feature. | ||
|
||
* To test the changes made, build the image and push it to quay. Replace [here](https://github.com/grafana/loki/blob/master/operator/config/overlays/openshift/size-calculator/storage_size_calculator.yaml#L18) with your quay image to test the changes. | ||
* To test the changes made, build the image and push it to quay. Replace [here](https://github.com/grafana/loki/blob/v2.8.0/operator/config/overlays/openshift/size-calculator/storage_size_calculator.yaml#L18) with your quay image to test the changes. |
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.
As a best practice, avoid putting release numbers in URLs because they become stale. Since the latest code is in main, I would change this to main.
* To test the changes made, build the image and push it to quay. Replace [here](https://github.com/grafana/loki/blob/v2.8.0/operator/config/overlays/openshift/size-calculator/storage_size_calculator.yaml#L18) with your quay image to test the changes. | |
* To test the changes made, build the image and push it to quay. Replace [here](https://github.com/grafana/loki/blob/main/operator/config/overlays/openshift/size-calculator/storage_size_calculator.yaml#L18) with your quay image to test the changes. |
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.
Using a version here was done by intention (the 2nd point in the description):
use a stable version in case lines are referenced
This is to make sure that the referenced line 18 uses a stable reference. Otherwise, if this file is changed and something is added/removed above line 18 in this file, the link would then point to something different than when created initially. I have seen this much too often.
If you don't like using a version, a git commit would do too, having the same effect.
If you would want to always refer to the main
branch, the line reference should be removed and maybe only the image
option mentioned.
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.
Good point, line 18 might move around. Let's try something like this instead:
* To test the changes made, build the image and push it to quay. Replace [here](https://github.com/grafana/loki/blob/v2.8.0/operator/config/overlays/openshift/size-calculator/storage_size_calculator.yaml#L18) with your quay image to test the changes. | |
* To test the changes made, build the image and push it to quay. Replace [spec.template.spec.containers.image](https://github.com/grafana/loki/blob/main/operator/config/overlays/openshift/size-calculator/storage_size_calculator.yaml) with your quay image to test the changes. |
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.
Done. I opted for another wording as spec.template.spec.containers.image
would not be correct because containers
is a list.
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.
[docs team] LGTM
**What this PR does / why we need it**: Manual backport of #9080 to "latest" after a customer reported a 404 to the docs@grafana email today.
main
instead ofmaster
branch