-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Upgrade packages #3405
Upgrade packages #3405
Conversation
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 putting together these additional changes; I had a couple of questions/comments - see in-line below.
Otherwise this LGTM.
gulpfile.js
Outdated
@@ -269,6 +268,25 @@ function coveralls() { // 2nd arg is a dependency: 'test' must be finished | |||
.pipe(shell('cat build/coverage/lcov.info | node_modules/coveralls/bin/coveralls.js')); | |||
} | |||
|
|||
gulp.task('watch', function () { |
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.
this gulp task is redundant and can be removed; there is already a watch
function declared earlier in the file (see here).
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.
yes, removed
@@ -34,7 +34,6 @@ function newPluginsArray(browserstack) { | |||
'karma-es5-shim', | |||
'karma-mocha', | |||
'karma-chai', | |||
'karma-requirejs', |
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.
If you're removing karma-requirejs
plug-in from this file, can it also be removed from the package.json
? I don't see it referenced anywhere else in the project.
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.
yes, removed
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.
LGTM
* RAD-2356 Upgrade Mocha karma-webpack karma * RAD-2356 update readme to support node v6 or more * Remove webpack output logs while testing * move from gulp-webserver to gulp-connect * remove karma-require and redundant watch task
* RAD-2356 Upgrade Mocha karma-webpack karma * RAD-2356 update readme to support node v6 or more * Remove webpack output logs while testing * move from gulp-webserver to gulp-connect * remove karma-require and redundant watch task
* RAD-2356 Upgrade Mocha karma-webpack karma * RAD-2356 update readme to support node v6 or more * Remove webpack output logs while testing * move from gulp-webserver to gulp-connect * remove karma-require and redundant watch task
* RAD-2356 Upgrade Mocha karma-webpack karma * RAD-2356 update readme to support node v6 or more * Remove webpack output logs while testing * move from gulp-webserver to gulp-connect * remove karma-require and redundant watch task
Type of change
Description of change
Removing packages with vulnerabilities
on running npm audit package "open" had critical vulnerabilities its being used by by us and by gulp-webserver as its dependency.
Changes in Node version support
Since latest versions of those can only be run successfully in latest versions of node, We will be supporting node of version v6 or more.
Note: this pull req includes the changes in pull req (#3392) which is to update mocha, karma, karma-webpack