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

Add support for sinon v4 #111

Merged
merged 2 commits into from
Sep 28, 2017

Conversation

Yeti-or
Copy link
Contributor

@Yeti-or Yeti-or commented Sep 26, 2017

No description provided.

@domenic
Copy link
Collaborator

domenic commented Sep 26, 2017

As you can see, the test suite fails on Sinon v4, so this is not mergeable.

@Yeti-or
Copy link
Contributor Author

Yeti-or commented Sep 26, 2017 via email

@daffl
Copy link
Contributor

daffl commented Sep 26, 2017

Looks like another dependency (nise ) not being installed. I had a similar problem in #108. Adding it to devDependencies will probably fix it but I'm still not sure why this keeps happening.

@Yeti-or
Copy link
Contributor Author

Yeti-or commented Sep 27, 2017

I think it's a bug in npm and it was fixed in 5.4.2: https://github.com/npm/npm/releases/tag/v5.4.2

@Yeti-or
Copy link
Contributor Author

Yeti-or commented Sep 27, 2017

steps:

$ node --version
v8.5.0
$ npm --version
5.3.0
npm install
mocha — ok
npm install sinon@^3.0.0
mocha — fail
Error: Cannot find module 'nise'

Because in nise package.json there is no more `main` field

@Yeti-or Yeti-or force-pushed the yeti-or.update_sinon_to_4.0.0 branch 5 times, most recently from a3537f3 to 89ab325 Compare September 27, 2017 23:35
@Yeti-or
Copy link
Contributor Author

Yeti-or commented Sep 27, 2017

@domenic tests are green, so is it mergeable now ?

@griest024
Copy link

@Yeti-or it could be related to sinonjs/nise#10

@domenic
Copy link
Collaborator

domenic commented Sep 28, 2017

Nice hack. Can we remove lolex too then?

@Yeti-or Yeti-or force-pushed the yeti-or.update_sinon_to_4.0.0 branch from 89ab325 to 5368706 Compare September 28, 2017 13:37
@Yeti-or
Copy link
Contributor Author

Yeti-or commented Sep 28, 2017

@domenic yes we can

@domenic domenic merged commit 3503cb7 into chaijs:master Sep 28, 2017
@Yeti-or Yeti-or deleted the yeti-or.update_sinon_to_4.0.0 branch September 28, 2017 16:40
fatso83 added a commit to fatso83/sinon that referenced this pull request Oct 4, 2017
franck-romano pushed a commit to franck-romano/sinon that referenced this pull request Oct 1, 2019
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.

5 participants