upgrade to sinon 5.x

[43081j]

just bumping sinon to 5.x.

Fixes #228.

the types aren't quite right but they work for us it seems (they get fixed in DefinitelyTyped/DefinitelyTyped#25491).

is this fine? wct was an odd one, it still has @polymer/sinonjs.. do we need that still?

cc @rictic

[merlinnot]

So you work on types for Sinon and you contribute to Polymer/tools? Much love mate ❤️

[43081j]

FYI the sinon 5.x types have been merged now.

ill try rebase this at the weekend and pull in the new types. though because lock files are such a pain to fix conflicts in, i may just start again 🙈

[merlinnot]

You can always regenerate lockfiles :)

[43081j]

turns out npm automatically resolves git conflicts in package-lock.json files as long as your package.json is resolved!

tests pass but the diff on the lock files looks unusually different considering the small footprint of my change. if i regen them all the additions seem to stick around so i guess its right?

cc @rictic maybe? to get this merged/checked

[merlinnot]

@43081j AppVeyor

[43081j]

doesn't look like the result on my changes:

info: [cli.install]    Installing npm dependencies...
error: [cli.main]   cli runtime exception: Error: Command failed: npm install
npm ERR! code 1
npm ERR! Command failed: C:\Program Files\Git\cmd\git.EXE submodule update -q --init --recursive
npm ERR! C:\Program Files\Git\mingw64/libexec/git-core\git-submodule: line 19: .: git-sh-setup: file not found
npm ERR! 

something specific to appveyor or how we have it configured maybe?

[43081j]

@justinfagnani can you take a look at this when you have time?

looks like the appveyor build fails for an unrelated reason but looks good otherwise.

[justinfagnani]

@43081j is this a breaking change?

[43081j]

@justinfagnani on our end? i don't think so as only tests and dev dependencies should have been altered

its a major version bump of sinon but this PR shouldn't affect any non-test code we have.

[justinfagnani]

I'm wondering if for test code that uses sinon as provided will break, which would require a major-version bump of wct-browser-legacy

[merlinnot]

It has it's own package.json: https://github.com/Polymer/tools/blob/master/packages/web-component-tester/wct-browser-legacy/package.json

[43081j]

ah you have a fair point @justinfagnani

fortunately i think i left that package alone? i did update the one in the main WCT package but not legacy afaik.

[43081j]

@justinfagnani any chance we can get this merged? if i need to change something to avoid needing a breaking change, let me know.

just want to avoid the pr going stale

[TimvdLippe]

Please do not update the packages of web-component-tester. I am working on that in a different PR and it involves breaking changes. The upgrades to (dev-)dependencies in the other packages are okay

[43081j]

@TimvdLippe i reverted the wct changes

i didn't change my original commit so i can keep the code changes locally

[TimvdLippe]

Thanks!

[ernsheong]

Please do not update the packages of web-component-tester

@TimvdLippe Any update regarding web-component-tester? Still stuck in 1.x, it's 6.x now

[TimvdLippe]

That update is being done separately atm. See the work on wct-mocha et al in other PRs.

[osamamaruf]

Till the sinon version is bumped should't #228 be kept open? (its 7.x now)