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 solve line endings cross-OS (merge after other PRs) #2484

Merged
merged 1 commit into from
Dec 5, 2015

Conversation

TimvdLippe
Copy link
Contributor

When checking out Polymer on a Linux machine, several files will be completely added due to non-CRLF endings. Therefore if you edit just 1 line, the whole file will be triggered as changed.

This change prevents the git diff from being cluttered, uniforming all line-endings across the project.
Fixes #2592

@samccone
Copy link
Contributor

I am 👍 to this thanks @JeremybellEU :)

@TimvdLippe
Copy link
Contributor Author

Thanks for your support @samccone . For future pull-requests in the affected files (total of 58 files), will generate full on diffs. If you want to prevent this, all files have to be fixed before that. However, in order to prevent merge issues, this should be done when merging. Because every single change in one of those 58 files (which is very very likely) will cause merge conflicts which are very hard to solve. Maybe the full diffs would be more suitable, to also keep git blame sane.

@samccone
Copy link
Contributor

yeah @JeremybellEU I think it would be wise to go ahead and correct the line endings across the project, but I am going to defer to the maintainers

@TimvdLippe
Copy link
Contributor Author

👍

@TimvdLippe
Copy link
Contributor Author

Finally found time to create the issue: #2592

@ebidel
Copy link
Contributor

ebidel commented Oct 18, 2015

@JeremybellEU Thanks for this. We need it :) I think we'll need to wait to merge it until the other PRs are addressed. We're going to go through those on a weekly basis now. cc @tjsavage

@ebidel ebidel changed the title Add .gitattributes to solve line endings cross-OS Add .gitattributes to solve line endings cross-OS (merge after other PRs) Oct 18, 2015
@dfreedm dfreedm self-assigned this Nov 6, 2015
@dfreedm
Copy link
Member

dfreedm commented Nov 6, 2015

LGTM, but I'll hold off merging until after the next release.

dfreedm added a commit that referenced this pull request Dec 5, 2015
Add .gitattributes to solve line endings cross-OS (merge after other PRs)
@dfreedm dfreedm merged commit 2b55a55 into Polymer:master Dec 5, 2015
@dfreedm
Copy link
Member

dfreedm commented Dec 5, 2015

I'm reverting this patch for now. Too awkward to handle the file changes piecemeal. If we decide to merge this again, it will be with every file normalized at once.

@TimvdLippe
Copy link
Contributor Author

Yes merging this causes quite a headache if there are a lot of open PRs. I would suggest to only merge this if there is some sort of code-freeze/very few PRs. Then the merges are managable, right now with 40 open PRs it is probably a nightmare.

For now I would advise all developers to set git config --global core.autocrlf input as described in https://help.github.com/articles/dealing-with-line-endings/#global-settings-for-line-endings . Therefore whenever the developer commits, just these files will have the correct line endings. This creates incremental fixing of the line endings and makes merging a lot easier.

Off Topic: This whole line endings is just a pain if not done pre-emptively and git can't really deal with them nicely.

samccone added a commit to samccone/polymer that referenced this pull request Dec 9, 2015
@samccone samccone mentioned this pull request Dec 9, 2015
@TimvdLippe TimvdLippe deleted the fix-line-endings branch February 5, 2016 13:50
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.

5 participants