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 grunt move to npm scripts #5592
Conversation
https://github.com/videojs/video.js/blob/master/Gruntfile.js can be removed as well |
We need to comb through out docs and remove grunt and update to npm commands |
d722654
to
119ee74
Compare
2cee6b5
to
f22d344
Compare
57365dc
to
351f39f
Compare
5dc3daf
to
4c7fe03
Compare
6235fd6
to
3be4c7b
Compare
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.
I think the karma-config update didn't get rebased properly
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.
So, overall, npm start
, npm test
, and npm run build
work.
package.json
Outdated
"postclean": "shx mkdir -p ./dist/lang ./test/dist", | ||
"changelog": "conventional-changelog -p videojs -i CHANGELOG.md -s", | ||
"build": "npm-run-all build-dev minify zip", | ||
"build-dev": "npm-run-all clean lint-errors build:js build:css build:lang build:copy build:test", |
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.
I would move build:test
just to npm test
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.
While that kind of makes sense, I think these should be bulit here so that they exist for the dev server.
package.json
Outdated
"postclean": "shx mkdir -p ./dist/lang ./test/dist", | ||
"changelog": "conventional-changelog -p videojs -i CHANGELOG.md -s", | ||
"build": "npm-run-all build-dev minify zip", | ||
"build-dev": "npm-run-all clean lint-errors build:js build:css build:lang build:copy build:test", |
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.
build:copy
only needs to run on publish, there's nothing in there that's used for dev
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.
maybe it's fine to leave it to match current functionality
Also, I realized that there are grunt tasks that were being used but not currently set: |
also, |
f099a3d
to
2cf74f6
Compare
Last grunt tasks are ported via this PR #5634 |
…5634) * cr * remove bin, migrate cdn-links * add back sandbox as script * sandbox script * port over check-translations * remove cdn-links * remove no longer used grunt tasks * update npm-run-all * progress from regular rollup build * regenerator package-lock * no progress on travis
Depends on #5589
Remove grunt altogether for npm scripts.