Skip to content
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

Revise Service concept, part 14 #39499

Merged
merged 2 commits into from
Feb 21, 2023

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Feb 16, 2023

These are some miscellaneous changes that I recommend making but that don't really fit in to any other PR.
preview / original

Split out from #30817
View that PR to see these changes in context.

These changes come after what you can see in #38562, and should not conflict with the changes I'm recommending in #38563.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 16, 2023
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 16, 2023
@netlify
Copy link

netlify bot commented Feb 16, 2023

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 5c775fc
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/63f35ab08e65d50008675ee7
😎 Deploy Preview https://deploy-preview-39499--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@shannonxtreme shannonxtreme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions and points of ambiguity, but overall clean change. Thanks for splitting!

Comment on lines 74 to 77
A Service in Kubernetes is a REST object (just as a Pod or a ConfigMap is an object
that you can access as a resource in the Kubernetes HTTP API.
You can use the HTTP API to create a new Service instance, to apply changes to an
existing Service, or to request deletion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A Service in Kubernetes is a REST object (just as a Pod or a ConfigMap is an object
that you can access as a resource in the Kubernetes HTTP API.
You can use the HTTP API to create a new Service instance, to apply changes to an
existing Service, or to request deletion.
A Service, similar to Pods and ConfigMaps, is an object
that you can access as a resource in the Kubernetes HTTP API.
You can use the HTTP API to create a new Service, to apply changes to an
existing Service, or to request deletion.

Removed "instance" - stick to "object" or "resource".

Also, is mentioning HTTP API necessary in both sentences?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wary of comparing Service (singular) with Pods and ConfigMaps (both plural). I also want to avoid starting the sentence with “Service” because I want the emphasis that the initial capital provides.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "similar to a Pod or a ConfigMap," ?

content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
@@ -94,12 +97,12 @@ spec:
targetPort: 9376
```

This specification creates a new Service object named "my-service", which
targets TCP port 9376 on any Pod with the `app.kubernetes.io/name=MyApp` label.
That manifest creates a new Service named "my-service", which
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
That manifest creates a new Service named "my-service", which
This manifest creates a new Service named "my-service", which

"This" feels like it more organically refers to "The preceding manifest"

content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
content/en/docs/concepts/services-networking/service.md Outdated Show resolved Hide resolved
@sftim sftim force-pushed the 20230216_service_update_part_14 branch 3 times, most recently from 375fe3a to 98c7d48 Compare February 17, 2023 11:40
Co-authored-by: Shannon Kularathna <[email protected]>
@sftim sftim force-pushed the 20230216_service_update_part_14 branch from 98c7d48 to c322487 Compare February 17, 2023 11:42
@tengqm
Copy link
Contributor

tengqm commented Feb 18, 2023

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2023
Co-authored-by: Shannon Kularathna <[email protected]>
@reylejano
Copy link
Member

Page preview: https://deploy-preview-39499--kubernetes-io-main-staging.netlify.app/docs/concepts/services-networking/service/

Imo, this PR improves the service concept page

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 21, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2fae5fe9c8379916c2f3919c2b30ba5d963541a2

@k8s-ci-robot k8s-ci-robot merged commit c67a986 into kubernetes:main Feb 21, 2023
@sftim sftim deleted the 20230216_service_update_part_14 branch February 21, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants