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

Correction for one small typo in content\en\docs\concepts\services-networking\service.md #18238

Closed
wants to merge 1 commit into from

Conversation

jmyung
Copy link
Member

@jmyung jmyung commented Dec 23, 2019

from #18237

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 23, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign kbarnard10
You can assign the PR to them by writing /assign @kbarnard10 in a comment when ready.

The full list of commands accepted by this bot can be found 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 language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Dec 23, 2019
@anshprat
Copy link
Contributor

anshprat commented Dec 23, 2019

Though this commit correctly fixes the typo, it touches other lines, new line changes etc ( I am assuming due to IDE being used). Is there a guideline on the IDE to be used for this project?
@sftim I had the same issue for #18231 PR. Thoughts on whats the right way to handle such IDE lead space / new line changes? I fixed it using a non-IDE editor to avoid any IDE changes.

@jmyung
Copy link
Member Author

jmyung commented Dec 23, 2019

Thanks for letting me know. @anshprat

I've used atom editor for now.
As you mentioned, it removed other white spaces automatically, but it never changed the line position of contents.
For my opinion, I think that this is ok, because markdown doesn't allow additional white spaces.

@anshprat
Copy link
Contributor

@jmyung - I also use atom editor :)
From a reviewer's perspective, having unrelated changes in a PR is always a red flag. However I will let more experienced folks chime in. Think of it like this - this may show as a diff for original authors and their IDE may again change these back.

@jmyung
Copy link
Member Author

jmyung commented Dec 24, 2019

close this.
I opened another PR. #18247

@jmyung jmyung closed this Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants