Update debug-service.md's tasks file to remove $ from kubectl and turn questions into bullet list #12798
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign @zacharysarah |
|
Deploy preview for kubernetes-io-master-staging ready! Built with commit 5b4c0fa https://deploy-preview-12798--kubernetes-io-master-staging.netlify.com |
| @@ -7,8 +7,7 @@ title: Debug Services | |||
| --- | |||
|
|
|||
| {{% capture overview %}} | |||
| An issue that comes up rather frequently for new installations of Kubernetes is | |||
| that a `Service` is not working properly. You've run your `Deployment` and | |||
| An issue that comes up rather frequently for new installations of Kubernetes is that a `Service` is not working properly. You've run your `Deployment` and | |||
| created a `Service`, but you get no response when you try to access it. | |||
| This document will hopefully help you to figure out what's going wrong. | |||
There was a problem hiding this comment.
is just cosmetic and not have a line with a single world in it. I know and understand from UX there is no difference however it just flows when you're in edit mode so to speak.
you agree ?
There was a problem hiding this comment.
@DanyC97 Consider two things:
- Scope creep
As you say, the line change is cosmetic rather than falling within the original scope of the proposed changes. Isolation of concerns is a good thing to achieve here.
- Consistency
There are other lines where you changed content but didn't also remove line breaks in sentences. If you're going to make cosmetic changes, I'd like to see them consistently applied, at least within the scope of the PR if not the whole document. (And again, scope creep.)
I think removing arbitrary line breaks is a helpful change; I just want to see it applied consistently as a separate PR. Make sense?
There was a problem hiding this comment.
ack, will stick with the initial scope and follow up with the cosmetics on a different PR
|
any help here with reviewing please ? |
There was a problem hiding this comment.
@DanyC97 Thanks for opening this PR. This looks good! I have two pieces of feedback:
- This comment about scope creep and consistency
- Be careful of using "fix" in the PR description; it's a GitHub magic word and will close Task topic - consistency around having full url to examples #12740, which I see you're using as an umbrella issue, before all of its component topics are addressed.
DanyC97
force-pushed
the
tasks-debug-app-cluster-dsvc
branch
from
1383444 to
ecc9447
Compare
DanyC97
changed the title
k8s-ci-robot
added
size/S
|
@zacharysarah please help review, PR re-done following previous comments. |
DanyC97
force-pushed
the
tasks-debug-app-cluster-dsvc
branch
from
ecc9447 to
1329841
Compare
k8s-ci-robot
added
size/M
DanyC97
changed the title
…n some questions into bullet list
DanyC97
force-pushed
the
tasks-debug-app-cluster-dsvc
branch
from
1329841 to
5b4c0fa
Compare
|
gentle reminder, thanks |
|
@DanyC97 Thanks for incorporating feedback. /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zacharysarah 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
…n some questions into bullet list (kubernetes#12798)
…n some questions into bullet list (kubernetes#12798)
…n some questions into bullet list (kubernetes#12798)
This PR bring the file code in line with the style guide where i've deleted
$in front ofkubectlThis PR is one of the #12740 's tasks