Skip to content

Normalize EOL before diff #335

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

Closed
wants to merge 1 commit into from
Closed

Conversation

Geobert
Copy link
Contributor

@Geobert Geobert commented Nov 19, 2017

This fixes most of the tests, see #241 for the remaining failing tests

@epage
Copy link
Member

epage commented Nov 21, 2017

So you found that nearly all the tests fail with autocrlf enabled.

The only reason I can think of this happening is we are somehow inserting our own newlines into the content. I think this is a problem in of itself and we should investigate and fix that.

In that case, this change could be masking a problem. Once that is addressed, should this go in? I'm mixed.

  • This could mask future problems with newlines.
  • How much do we care about people without autocrlf messing up the content of the tests?

I think my preference is still towarsds editorconfig and maybe gitattribute.

@Geobert
Copy link
Contributor Author

Geobert commented Nov 22, 2017

You're right, this normalization shouldn't be done on the test, but on the generated files

@Geobert Geobert closed this Nov 22, 2017
@epage
Copy link
Member

epage commented Nov 28, 2017

I'm sorry that solving this hasn't been as straightforward as this originally seemed :(

@Geobert
Copy link
Contributor Author

Geobert commented Nov 28, 2017

Don't worry, it's the correct decision ^^ That's why reviews are for ^^

@Geobert Geobert deleted the fix#241 branch November 28, 2017 21:20
@epage epage mentioned this pull request Jan 25, 2018
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.

2 participants