Update debug-service.md's tasks file to remove $ from kubectl and turn questions into bullet list#12798
Conversation
|
/assign @zacharysarah |
|
Deploy preview for kubernetes-io-master-staging ready! Built with commit 5b4c0fa https://deploy-preview-12798--kubernetes-io-master-staging.netlify.com |
| 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 ? |
zacharysarah
left a comment
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 #12740, which I see you're using as an umbrella issue, before all of its component topics are addressed.
1383444 to
ecc9447
Compare
|
@zacharysarah please help review, PR re-done following previous comments. |
ecc9447 to
1329841
Compare
…n some questions into bullet list
1329841 to
5b4c0fa
Compare
|
gentle reminder, thanks |
|
@DanyC97 Thanks for incorporating feedback. /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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…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