Skip to content

update dependencies to fix known vulnerabilities in them #2

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

striezel
Copy link

@striezel striezel commented Jul 3, 2021

Updating dependencies to fix vulnerabilities. :)
This also solves #1.

When this is merged, it is probably a good idea to publish a new release of default-compare on NPM.

@striezel
Copy link
Author

@doowb
AppVeor CI fails, beause yargs (a dependency of the newer Mocha version) requires at least Node.js 10, but the tests on AppVeyor run with older versions of Node:

Error: yargs parser supports a minimum Node.js version of 10. Read our version support policy: https://github.com/yargs/yargs-parser#supported-nodejs-versions
    at Object.<anonymous> (C:\projects\default-compare\node_modules\yargs-parser\build\index.cjs:991:15)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (C:\projects\default-compare\node_modules\mocha\lib\cli\options.js:12:21)
    at Module._compile (module.js:653:30)
npm ERR! Test failed.  See above for more details.

Not sure how you prefer to handle this, but I guess it should be a major version bump for default-compare then.
I could also update the Node.js versions in the AppVeyor configuration, if you don't prefer to do that yourself. Please let me know how to proceed here.

@doowb
Copy link
Owner

doowb commented Aug 11, 2021

Hi @striezel , sorry I missed this earlier...

Thank you for the PR, but [email protected] is not one of the versions affected by that advisory. It's not necessary to update this package yet.

Aside from that, if this package needed to update that dependency, I'd do that first, without updating mocha. If you want to remove the update to mocha, I'll leave this PR open and merge it in the next time something else in this package needs to be updated.

@striezel striezel force-pushed the update-dependencies branch from d725a80 to 3594187 Compare August 13, 2021 00:05
@striezel
Copy link
Author

Alright.
I've removed the update to mocha and force-pushed the change. That way the tests should pass on AppVeyor CI, too.

And indeed they do pass: https://ci.appveyor.com/project/doowb/default-compare/builds/40363391 :)

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.

2 participants