Skip to content

[SPARK-28842][DOC]Cleanup the formatting/trailing spaces in the K8s integration testing guide#25682

Closed
alaazbair wants to merge 1 commit intoapache:masterfrom
alaazbair:SPARK-28842
Closed

[SPARK-28842][DOC]Cleanup the formatting/trailing spaces in the K8s integration testing guide#25682
alaazbair wants to merge 1 commit intoapache:masterfrom
alaazbair:SPARK-28842

Conversation

@alaazbair
Copy link

What changes were proposed in this pull request?

Removing the trailing spaces in resource-managers/kubernetes/integration-tests/README.md

Why are the changes needed?

Improve the formatting of the K8s integration testing guide

Does this PR introduce any user-facing change?

No

How was this patch tested?

I have run all tests with ./dev/run-tests but no tests were added because the only change I did was on the README file and I only removed spaces and one empty line.

@alaazbair
Copy link
Author

alaazbair commented Sep 5, 2019

@holdenk Could you please review my PR.

@srowen
Copy link
Member

srowen commented Sep 13, 2019

Why would this matter though? they do not affect the rendered doc.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@dongjoon-hyun
Copy link
Member

Hi, @alaazbair . Thank you for making a PR, but shall we close this PR if there is no justification?

@alaazbair
Copy link
Author

alaazbair commented Sep 17, 2019

Hi @srowen @dongjoon-hyun , sorry for the late response. I think this change will matter the most when between commits we get diffs polluted with unchanged lines (during a change in the documentation, an existing trailing space was removed ). In addition trailing spaces could break some text editors, for instance pressing END when the cursor is on a line containing trailing spaces, will not move the cursor to the end of the line and this can be annoying if the formatting isn't consistent throughout the file.

Besides this I can't think of any other justification. if you think this change isn't justified, I will gladly close the PR.

Cheers.

@srowen
Copy link
Member

srowen commented Sep 17, 2019

Generally I wouldn't make this change; IDEs shouldn't be adding/removing whitespace automatically for just this reason. The other downside is the very small additional chance of spurious merge conflicts, if we try to back-port a doc change that conflicts.

That said, for a first-time contributor we're a lot more open to small changes, and this only affects one file. (By the same token, this won't affect any other .md files, and I don't think we'd change them all).

@alaazbair alaazbair closed this Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants