Remove duplicate references to Docker Engine legacy container feature in "Service" page #44274
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
language/en
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
/retitle Remove duplicate references to Docker Engine legacy container feature in "Service" page |
k8s-ci-robot
changed the title
| (see [makeLinkVariables](https://github.com/kubernetes/kubernetes/blob/dd2d12f6dc0e654c15d5db57a5f9f6ba61192726/pkg/kubelet/envvars/envvars.go#L72)) | ||
| that are compatible with Docker Engine's | ||
| "_[legacy container links](https://docs.docker.com/network/links/)_" feature. | ||
|
|
There was a problem hiding this comment.
nit: Extra blank line isn't needed. Not a big deal since we don't do mdlint enforcement, but if you need to update for some other reason it would be nice to clean it up.
There was a problem hiding this comment.
Ok, @stmcginnis thanks for suggestion will do!
k8s-ci-robot
added
the
lgtm
|
/assign @sftim |
There was a problem hiding this comment.
Thanks for the PR @rohan-kapse, but I don't see strong evidence that issue #44272 is a valid bug report.
I think merging this PR would remove some accurate information and not overall be helpful.
/lgtm cancel
If the text you're removing is accurate, that doesn't mean it's easy to understand. A different PR that helps readers understand the two different kinds of environment variables might help.
If you want to take that approach, you can. The page seems correct as it is so making improvements here is a task for someone comfortable with technical writing.
k8s-ci-robot
removed
the
lgtm
|
Maybe I'm wrong in my review. If so, please reply to let me - and other contributors - know. |
|
No prob, @sftim I just thought it addressed the same reference twice so I thought that only having one would be feasible. But it surely makes more sense from your point. |
|
Got it. This is the text we're keeping: Kubernetes also supports and provides variables that are compatible with Docker
Engine's "_[legacy container links](https://docs.docker.com/network/links/)_" feature.
You can read [`makeLinkVariables`](https://github.com/kubernetes/kubernetes/blob/dd2d12f6dc0e654c15d5db57a5f9f6ba61192726/pkg/kubelet/envvars/envvars.go#L72)
to see how this is implemented in Kubernetes./lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 7ca7dd3070fe0b8826ec62a3c9ec4e219ab53511
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sftim 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 |
k8s-ci-robot
added
the
approved
Removed duplicated wording reference to Docker Engine's legacy container links #44272