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

Move the ava-files package back into core #1196

Merged
merged 4 commits into from Jan 19, 2017

Conversation

forresst
Copy link
Contributor

Fixes #1192

@@ -44,7 +44,7 @@
},
"scripts": {
"test": "xo && nyc tap --no-cov --timeout=150 test/*.js test/reporters/*.js",
"test-win": "tap --no-cov --reporter=classic --timeout=150 test/*.js test/reporters/*.js",
"test-win": "tap --no-cov --reporter=classic --timeout=150 test/ava-files.js test/reporters/*.js",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to change this back ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oups ! I'm unmasked, I test under windows!

@@ -0,0 +1,180 @@
const path = require('path');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put 'use strict'; above here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep Sir !

@sindresorhus sindresorhus changed the title Move ava-files back into core Move the ava-files package back into core Jan 18, 2017
@sindresorhus
Copy link
Member

There are some lint issues you need to fix. See Travis.

@sindresorhus
Copy link
Member

Some tests are failing: https://travis-ci.org/avajs/ava/jobs/193048227

@forresst
Copy link
Contributor Author

@sindresorhus I will look at these tests quickly. Sorry

There is a second point to consider: spread for Node 4 (https://travis-ci.org/avajs/ava/jobs/193048228#L231).
I know spread does not work without the flag under node 4. It works on the avajs/ava-files repository but not under avajs/ava. Is there a tip or parameter under travis for this to work?
Finally, should we use spread in avajs/ava?

@sindresorhus
Copy link
Member

No, only use syntax supported on Node.js 4.

@novemberborn
Copy link
Member

It works on the avajs/ava-files repository but not under avajs/ava.

I assume it's used in the old tests? We're using AVA to run those tests, and of course AVA takes care of transpiling the spread operator. That's not the case with tap.

@forresst
Copy link
Contributor Author

@sindresorhus ok, i will remove spread

@novemberborn Obviously, how could I miss out on this? I will read the doc in French! 😄

remove spread for comptability node 4
@forresst
Copy link
Contributor Author

All tests passing. Sorry for my mistakes.

@mightyiam
Copy link
Contributor

No, only use syntax supported on Node.js 4.

🥇 tough leadership decisions

@novemberborn novemberborn merged commit eccd823 into avajs:master Jan 19, 2017
@novemberborn
Copy link
Member

This is awesome. Thanks @forresst!

sindresorhus added a commit that referenced this pull request Jan 19, 2017
@forresst forresst deleted the move-ava-files-back branch January 26, 2017 10:27
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

4 participants