-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Replaced unavailable image in https://k8s.io/docs/tutorials/stateless-application/guestbook/ #44290
Conversation
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Changed the wrong file at first, but fixed it now. |
@@ -17,7 +17,7 @@ spec: | |||
spec: | |||
containers: | |||
- name: php-redis | |||
image: gcr.io/google_samples/gb-frontend:v5 | |||
image: corelab/gb-frontend:v5 |
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 get that we'd like to suggest an alternative.
- Why is this container image a good choice / could we switch to another image (maybe one that we, the
Kubernetes project, control)? - please specify the registry;
corelab
is not a DNS name that resolves.
I know you mean Docker Hub, but some readers might not.
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.
A better alternative may be us-docker.pkg.dev/google-samples/containers/gke/gb-frontend:v5, which is now the image used in the original tutorial.. Pasting the link in a browser leads to a Google cloud container registry.
A note could also be added at the top of the .yaml file directing readers to the original tutorial's yaml file if they have run into any trouble with the container image.
If this works, I will update the PR accordingly.
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.
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.
Sounds good. I've corrected the page in a new commit @adityasamant25
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.
@azn-abel Looks good to me. However I don't have the permissions to add the lgtm label. Can you please request one of the other reviewers to help?
Also would be great if you could squash the commits into one.
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.
@adityasamant25 Thanks - I've squashed the commits. I'll request @sftim for the lgtm label.
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 good to me.
Per #44290 (comment)
We'll need an answer to that question. |
|
/lgtm |
LGTM label has been added. Git tree hash: 1204b0730ae3d05912c9f67c076df97c8c675f87
|
/assign @natalisucks for approval |
It would be great to expedite the approval for this PR as it fixes an issue that is repeatedly being reported by multiple users. |
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 fixed now!
pinging @natalisucks for approval |
Hi folks – SIG Docs has PR wranglers assigned each week in order to move through the contribution queue of PRs, of which the schedule can be found here: https://github.com/kubernetes/website/wiki/PR-Wranglers Feel free to ping the relevant PR wrangler in a PR, or in the #sig-docs channel on Kubernetes Slack, to have your work looked at as needed. I can see with the scheduling that Tim has already given his review, and I'm on schedule next week. Unfortunately, the assigned reviewers have yet to respond. /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adityasamant25, natalisucks The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
resolve #44190