chore: Update to use plugin generator v7.0.2 #28
Conversation
brandonocasey
force-pushed
the
chore/generator-v7
branch
from
d37f345 to
c918645
Compare
misteroneill
changed the title
brandonocasey
force-pushed
the
chore/generator-v7
branch
from
a859b03 to
dfe983f
Compare
scripts/karma.conf.js
Outdated
| // which means the travis config in this repo is also | ||
| // different | ||
| config.plugins.push('karma-coverage'); | ||
| config.browsers = ['ChromeHeadless']; |
There was a problem hiding this comment.
We can use the vjs karma defaults here to be more consistent (and update the travis file). This was only running in headless Chrome because it's a relatively vanilla library.
There was a problem hiding this comment.
OK I will revert this and the travis file to the defaults
.travis.yml
Outdated
| addons: | ||
| chrome: stable | ||
| firefox: latest | ||
| apt: |
There was a problem hiding this comment.
You should be able to simplify this to chrome: stable
.travis.yml
Outdated
| before_script: | ||
| - export CHROME_BIN=/usr/bin/google-chrome | ||
| - export DISPLAY=:99.0 | ||
| - sh -e /etc/init.d/xvfb start |
There was a problem hiding this comment.
Is this still necessary or does the vjs karma config run on normal Chrome and not headless by default?
There was a problem hiding this comment.
Not sure, let me check
package.json
Outdated
| "precommit": "lint-staged" | ||
| "prepublish": "not-in-install && npm run build || in-install && npm-merge-driver install", | ||
| "precommit": "lint-staged", | ||
| "posttest": "shx cat test/dist/coverage/text.txt" |
There was a problem hiding this comment.
I think files related to the build should live outside of dist/ so they don't get published to npm. Perhaps package.files should only have dist as well?
There was a problem hiding this comment.
test/dist is where we currently build the test bundle, and we already gitignore it. I think it made logical sense to put coverage next to the test dist as well.
There was a problem hiding this comment.
Misread test/dist as dist/, ignore my comment
.travis.yml
Outdated
| language: node_js | ||
| node_js: | ||
| - 8 | ||
| - 'lts/*' |
There was a problem hiding this comment.
Looks like travis should automatically pick up the nvmrc https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#specifying-nodejs-versions-using-nvmrc
|
@forbesjo ready for a re-review when you have the time, should have everything we talked about. Thanks for digging into optimizations for the travis config. |
Changes