-
Notifications
You must be signed in to change notification settings - Fork 502
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
WIP: removal of trailing whitespace #257
base: master
Are you sure you want to change the base?
Conversation
Thank you! The problem with white-space fixes is that they make git blame much harder to use and there is nothing that prevents trailing whitespace from being introduced again in the future. So I would prefer if you could revert those. |
Would you take the whitespace fixes if we added a CI step to check for them? I've been hacking on an unrelated feature myself and my editor keeps complaining about them. |
Yeah, I guess so. But please still put them in a separate commit. |
I separated the pull request into individual commits, and additionally added an .editorconfig file so that trailing whitespace doesn't happen in the future. |
I'll look into setting up a CI step today to enforce code style. |
I'd also hold off on merging this for the moment, we can bundle any/all formatting fixes together and then put the commit hashes in a .git-blame-ignore-revs file Actually never mind, we can add the formatting commits to the ignore file whenever |
I don't think the Can we split this into one pull request for whitespace + ignore file + CI changes, and a second one for the other changes? |
Split into #258 |
Rebased on latest master. Leaving this PR open in case the .editorconfig file is considered useful. Also still looking into how to add these particular fixes in an automated way. |
* Trim trailing whitespace always * Use linux-style newlines (LF, not CRLF)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be merged independently of a repo-wide reformatting and CI check.
Removed lines won't show up in git blame :)
No description provided.