-
Notifications
You must be signed in to change notification settings - Fork 29
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
Refactor and fix a couple of issues #33
Conversation
And prefer boolean logic over conditionals
package.json
Outdated
@@ -28,7 +28,9 @@ | |||
"lib/", | |||
"!lib/**/*.test.js" | |||
], | |||
"dependencies": {}, | |||
"dependencies": { | |||
"array-from": "2.1.1" |
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.
Did you add this with the exact version number on purpose?
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.
Nope
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.
Pushed a temporary fix, I'll fold it in when we're ready to merge
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.
Solid work! Very nice 👍
212ac86
to
a58b284
Compare
And use polyfill, as IE11 doesn't support
IE11 [doesn't support collection arguments][1]. Disabling the check means that linting will pass, when it should not. Since the aim is to keep this library compatible with IE11, then the tests should be able to run in IE11. Instead of disabling the check for use of invalid use `new Set()`, replace invalid calls to `new Set()` with a helper `createSet()` that supports collection arguments. [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set
It is much faster in the runtimes we care about: * https://gist.github.com/mroderick/daec6d96e476048e60c2ba30408fbbed * https://jsperf.com/getindex-vs-indexof/1
Thank you :) |
This PR is a follow up to the discussion in sinonjs/referee#29.
It contains the same changes as in the branch mentioned in sinonjs/referee#29, just packaged slightly differently
I've tried to keep each commit focused on just one thing and have tried to give a decent order to the changes, to make it easier to follow.
Main themes
deepEqual
insinon
in preparation of consolidating everything insamsam
indexOf
instead of homegrown solutionHow to verify - mandatory
npm install
npm test
Checklist for author
npm run lint
passes