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

Add .gitattributes to normalize how line endings are stored #4240

Merged
merged 2 commits into from
May 11, 2022

Conversation

liamdon
Copy link
Contributor

@liamdon liamdon commented May 11, 2022

Related to issue #4238

Problem

Line endings in the repo are inconsistent between CRLF (Windows) and LF (Unix/Git).

Solution

Add a .gitattributes file so that git will normalize files as they are checked in, while still allowing developers to use their native line endings locally.

Issue #4238 suggests an additional cleanup task to fix existing files that were already checked in as CRLF. I am happy to open that task as a PR as well (just let me know), but I thought that the core maintainers might prefer to do so since it will be a scary diff (every line changed in 30 files).

Context

Currently around 5% of the source files are stored as CRLF (Windows), and the other 95% are LF (Unix) (see full report in #4238).

This causes issues for developers on both platforms:

  • It can be tricky to ensure your editor retains the existing line endings for a given file, but if you change a file's line endings the resulting diff will show every line as changed. This makes it hard to review the PR! (example)
  • Developers might also be confused about whether they should be changing line endings deliberately for consistency.
    For example, mat4.js and vec2.js use different line endings - should a developer change them to be consistent, or leave them as they are? What format should they use for a new file?

By using .gitattributes to normalize line endings at checkin/checkout, no-one will have to think about line endings ever again 🥳

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@willeastcott
Copy link
Contributor

This is great @liamdon - thanks so much. And yep, it would be a big help if you also attended to the 5% of files which use CRLF. 🙏

@willeastcott willeastcott merged commit 2c2610c into playcanvas:main May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants