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.

Sign up for GitHub

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

Jump to bottom

Move mocha to devdependencies #2

Merged
merged 3 commits into from
Oct 16, 2019
Merged

Move mocha to devdependencies #2

merged 3 commits into from
Oct 16, 2019

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Oct 15, 2019

(Basically the same as a PR I made over at upstream: swagger-api/swagger-node#591)

mocha is a testing framework, so it shouldn't be needed outside of devdependencies.

Having fewer dependencies obviously pulls in less dependencies, for those who use swagger in their project. That makes for a lighter node_modules folder, less things to keep up-to-date to satisfy npm audit and yarn audit, etc.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Oct 15, 2019 •
edited
Loading

Also, consider updating swagger-test-templates, since I made a couple of PRs that were merged at that package's repo updating its dependencies. If you were to merge this PR and update swagger-test-templates, all fixable npm audit / yarn audit vulnerabilities against this package would be resolved, if I understand correctly.

@DeeDeeG
Move mocha to devdependencies
8a83943 
It's vulnerable, per `npm audit`, so we don't want it in production.

Inspired by GitHub user makuro's commit @ 9045d25.

makuro/swagger-node@9045d25
@DeeDeeG DeeDeeG force-pushed the mocha-to-devdependencies branch from f0a327c to 8a83943 Compare October 16, 2019 13:39
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Oct 16, 2019

Hi, I fixed the merge conflict here.

@maziyarpanahi maziyarpanahi self-assigned this Oct 16, 2019
@maziyarpanahi maziyarpanahi added the enhancement New feature or request label Oct 16, 2019
@maziyarpanahi
Copy link
Member

Looks great, thanks @DeeDeeG! Testes have passed and I'll merge this. I'll bump the version to 0.8.3 and publish it on NPM soon.

@maziyarpanahi maziyarpanahi merged commit 42e8687 into multivacplatform:master Oct 16, 2019
@DeeDeeG DeeDeeG mentioned this pull request Oct 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@maziyarpanahi maziyarpanahi

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DeeDeeG @maziyarpanahi