Skip to content

Update .gitattributes and normalize all line endings#730

Merged
mousetraps merged 2 commits intomicrosoft:masterfrom
mousetraps:gitattributes
Mar 7, 2016
Merged

Update .gitattributes and normalize all line endings#730
mousetraps merged 2 commits intomicrosoft:masterfrom
mousetraps:gitattributes

Conversation

@mousetraps
Copy link
Copy Markdown
Contributor

Related to issue described in #727

@mjbvz can you pull this branch onto your machine first to verify this change resolves the issue?

@mousetraps
Copy link
Copy Markdown
Contributor Author

mousetraps added a commit that referenced this pull request Mar 7, 2016
Update .gitattributes and normalize all line endings
@mousetraps mousetraps merged commit 20085ab into microsoft:master Mar 7, 2016
@mjbvz
Copy link
Copy Markdown
Contributor

mjbvz commented Mar 8, 2016

Thanks! this should make future changes a lot easier (and also easier to review). Sorry for not taking a look too, I mixed up the name with the other changes that were similarly titled.

I believe the change may have introduced a regression with the tests though: https://ci.appveyor.com/project/mousetraps/nodejstools/build/1.0.147/job/4kgj98i6q666oj8b/tests

It looks like a number of Analysis tests are now failing ( #731 will disable all the repl window tests and some of the other ones that are failing). These also fail locally.

If this change is the root cause, we should open a bug to track fixing this. Reverting this change is probably a bad idea given the number of files touched, and hopefully the fix is as easy as adjusting a few numbers (but it'd be nice if the tests were more robust).

@mousetraps
Copy link
Copy Markdown
Contributor Author

Interesting - I actually ran those tests successfully on my machine, since several of them are sensitive to line endings (e.g. ParserTests.cs requires CRLF). Anyways, it appears my assumptions about how git automatically decides what's "best" were invalid, so I took another shot at it in #736.

In any case, I agree that the tests shouldn't be so sensitive.

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.

3 participants