Update dependencies. #253

Merged
merged 1 commit into from Aug 26, 2014

Projects

None yet

5 participants

@XhmikosR
Member

BTW, I keep getting 15/32 tests failing on Windows.

@sindresorhus sindresorhus merged commit e002480 into gruntjs:master Aug 26, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@UltCombo
Contributor

@XhmikosR your line endings checkout style is probably wrong. You should checkout with LF line endings.

@XhmikosR
Member

My line endings are fine but I'll look again later.
On Aug 26, 2014 10:35 PM, "Fabrício Matté" notifications@github.com wrote:

@XhmikosR https://github.com/XhmikosR your line endings checkout style
is probably wrong. You should checkout with LF line endings.


Reply to this email directly or view it on GitHub
#253 (comment)
.

@UltCombo
Contributor

@XhmikosR are you sure you had autocrlf = input in your global .gitconfig before cloning the repo, or have you fixed it in your local repo and did a reset/checkout?

@XhmikosR
Member

@UltCombo: yeah I do have autocrlf already enabled.

@XhmikosR
Member

Perhaps we should enforce LF via a .gitattributes file.

@UltCombo
Contributor

@XhmikosR sounds like a good idea! Maybe open another PR or issue?

@vladikoff
Member

sounds like a good idea! Maybe open another PR or issue?

Those should probably be in a global .gitattributes

@UltCombo
Contributor

@vladikoff why global? Windows folks may not always want to checkout with LF line endings in every single repository they contribute to. Modularity for the win.

@XhmikosR
Member

I just tried it.

A simple .gitattributes file in the repo with the following fixes the tests failures here

*.* text eol=lf
@UltCombo
Contributor

@vladikoff what's the problem with a local .gitattributes? It would be quite useful for Windows contributors AFAICS.

@sindresorhus
Member

*.* text eol=lf

This is a bad idea. It will make everything text, even binary files like images. It should only be applied to specific extensions.

@vladikoff
Member

Thanks to @shama, he added * text=auto just now

@sindresorhus
Member

@vladikoff * text=auto does nothing for this as it still checks out in CRLF, but converts on LF on commit.

@vladikoff
Member

@sindresorhus yeap okay, it's good to have .gitattributes in the repo though

@sindresorhus sindresorhus added a commit that referenced this pull request Aug 27, 2014
@sindresorhus sindresorhus force LF on JS files - #253 395be84
@shama
Member
shama commented Aug 27, 2014

@sindresorhus Why force line endings? We only care that the line endings when committed are unix. Users should get whatever they prefer, right?

@UltCombo
Contributor

@shama the unit tests fail when the fixtures contain \r\n.

Another option would be normalizing the \r\n to \n when reading the fixture files' contents.

@shama
Member
shama commented Aug 27, 2014

Now the tests are platform independent with e75540c which IMO is a better strategy than using git to force line endings.

@sindresorhus
Member

I don't agree. It's easier for everything to just stick to LF. All not completely awful Windows editors supports LF, so that's not a blocker either. But care.

@UltCombo
Contributor

It's easier for everything to just stick to LF. All not completely awful Windows editors supports LF, so that's not a blocker either.

Agreed.

Though, normalizing the line endings in the unit test script does not add much complexity and allows a bit more of flexibility. Anyway, either of the solutions are fine, and one does not conflict with the other so why bother.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment