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
chore: remove unnecessary dependencies #2863
Conversation
as we already have jasmine's most of dependencies also, refactor the test a bit
.eslintrc.json
Outdated
@@ -54,5 +52,6 @@ | |||
"pickRandomsFromList": true, | |||
"respecConfig": true | |||
}, | |||
"ignorePatterns": ["src/core/text-loader.js"], |
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.
Waiting on eslint/eslint#13196 to get import.meta.url
support. Can get rest of the PR in before that though, but no hurries.
@marcoscaceres I was thinking of also removing Also, do we need all the karma browser launchers? |
1f99cfa
to
e9b2a06
Compare
"eslint": "^6.8.0", | ||
"eslint-config-prettier": "^6.10.1", | ||
"eslint-plugin-import": "^2.20.1", | ||
"eslint-plugin-jasmine": "^4.1.0", | ||
"eslint-plugin-prettier": "^3.1.2", | ||
"handlebars": "^4.7.3", |
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.
We might need to bring this back if the Geonovum folks finish migrating over their profile... they are still using it, I think.
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.
@thijsbrentjens Would you be needing handlebars?
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.
Oh, maybe not... all the templates are using hyperHTML! https://github.com/Geonovum/respec/tree/develop/src/geonovum/templates
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.
As an aside, looks like downstream is getting really out of date... @thijsbrentjens, we should look at moving more stuff over, as Geonovum might be missing out on important features and security updates.
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.
Thanks for confirming!
"test:headless": "node ./tests/headless.js", | ||
"test:karma": "npm run karma", | ||
"postinstall": "opencollective-postinstall" | ||
"test:karma": "npm run karma" | ||
}, | ||
"dependencies": { | ||
"colors": "^1.4.0", | ||
"command-line-args": "^5.1.1", | ||
"command-line-usage": "^6.1.0", | ||
"epipebomb": "^1.0.0", |
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.
We might not need this anymore in newer node:
mhart/epipebomb#10
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.
We're supporting Node 12 presently though, as 14 isn't LTS yet.
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.
ah... true! Booooo!!!!
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.
few comments about some other things that could go... but overall looks good.
Co-authored-by: Marcos Cáceres <mcaceres@mozilla.com>
No description provided.