Skip to content

Enforce whitespace consistency in maintained files - #1489

Open
alex-thiessen-for-siemens wants to merge 3 commits into
jbeder:masterfrom
alex-thiessen-for-siemens:chore/enforce-whitespace-consistency
Open

Enforce whitespace consistency in maintained files#1489
alex-thiessen-for-siemens wants to merge 3 commits into
jbeder:masterfrom
alex-thiessen-for-siemens:chore/enforce-whitespace-consistency

Conversation

@alex-thiessen-for-siemens

@alex-thiessen-for-siemens alex-thiessen-for-siemens commented Sep 2, 2026

Copy link
Copy Markdown

Summary

This standalone cleanup is motivated by the incidental whitespace changes mixed into #1488. It normalizes the existing maintained-file baseline and adds a permanent check so future semantic PRs do not need to carry unrelated whitespace edits.

Policy

  • No trailing spaces or tabs.
  • Each line uses LF as its end-of-line character.
  • Non-empty files end with exactly one final LF.
  • The vendored test/googletest-1.16.0/ tree is excluded.

The cleanup normalizes maintained text to LF and leaves the vendored tree unchanged. You may run python3 util/check-whitespace.py to check whitespace before submitting a pull request.

CI

The new Whitespace workflow runs on pushes, pull requests, and manual dispatch. Please add the Whitespace check to the required status checks for master after this workflow is available. This account does not have permission to change the upstream branch rules.

This PR intentionally does not modify or comment on #1488.

@alex-thiessen-for-siemens
alex-thiessen-for-siemens force-pushed the chore/enforce-whitespace-consistency branch from b6431a5 to 4b6cf63 Compare September 2, 2026 17:35
@SGSSGene

SGSSGene commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

This is awesome! Thank you!

May I ask, which files end in CRLF? the only two that I can find are yaml-cpp.natvis and yaml-cpp.natvis.md

This should definitely be also fixed and changed to LF.

I would say the policy is:

  • No trailing spaces or tabs.
  • Each line ends with a newline (LF)
  • The vendored test/googletest-1.16.0/ tree is excluded.

@SGSSGene SGSSGene left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is great!

I would like to pin it down to only LF symbols.
And even though it is called a 'newline' character, I think it is easier/better to think of it as a 'end of line' character, which makes it very clear when/how to use it and why there has to be one at the end of a non-empty file.

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
@SGSSGene

SGSSGene commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

@jbeder I think this is a great change to keep maintain cleanliness of all files which reduces noise in PRs for whitespaces. This needs your approval and intervention to add there CI workflow as an requirement for PRs to pass.

Document trailing-whitespace and final-newline requirements.
Require LF line endings and configure editor defaults.
Keep the vendored GoogleTest sources excluded from the policy.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reject carriage returns so maintained files use LF line endings.
Retain trailing-whitespace, final-newline, binary, and vendored-tree
handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Normalize the maintained-file baseline, including remaining CRLF files.
Ensure all maintained text uses LF line endings.
Leave the vendored GoogleTest tree unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@alex-thiessen-for-siemens
alex-thiessen-for-siemens force-pushed the chore/enforce-whitespace-consistency branch from 4b6cf63 to 72ced0f Compare September 3, 2026 19:58
@alex-thiessen-for-siemens

Copy link
Copy Markdown
Author

Addressed. src/contrib/yaml-cpp.natvis and src/contrib/yaml-cpp.natvis.md were the only maintained files using CRLF; both now use LF. The policy and checker require LF for maintained text, while the vendored test/googletest-1.16.0/ tree remains excluded. Making Whitespace a required status check still needs an upstream maintainer with branch-rule permissions.

@alex-thiessen-for-siemens

Copy link
Copy Markdown
Author

Initially, I decided that natvis-related files should remain CRLF, as these are for Windows users only. Hope Visual Studio can deal with LF natvis, else an exception to the EOL check would be needed.

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.

2 participants