-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Remove lodash from tests #1323
Remove lodash from tests #1323
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great @urvalla, thanks! I'm a bit unsure regarding losing range
, for larger arrays we may want to introduce our own helper.
Are these all of the instances of lodash
? If so, could you also remove it from the package.json
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved as well, pending package.json update. thanks @urvalla!
Thanks for fast reviews! Lodash is not declared in package.json explicitly. It is a dependency of babel, eslint, etc. The Travis build has been terminated. That's weird a little bit, as "unit tests with coverage" in the same build are fine. |
Looks like mocha didn't exist because of an unresolved promise. I'm going to restart to build and see if it was a fluke, but it might be helpful to run https://github.com/mafintosh/why-is-node-running to figure out what promise isn't resolving |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build seems to be passing now. This looks good to me. Thanks @urvalla!
(cherry picked from commit d062352)
Extracted from #1242
π Description
npm run lint:fix
).