Add build step to CI #499
Add build step to CI #499
Conversation
Fixes #481. Adds the build step to Travis CI. Since we are using webpack v2, the build will exit with a non zero error code, if any build failures occur.
|
I think this is reasonable, but also worrying that our production build is different enough from what we're testing that we feel compelled to create separate tasks for them in CI. |
|
I also wonder if these tasks can be parallelized to speed up the Travis build. |
|
To the last point, we could consider: https://www.npmjs.com/package/concurrently |
|
Okay, so run the dev build as well and run them in parallel, along with the test suite? |
|
Should I create seperate npm scripts for each build, to make it a little cleaner looking? Or potentially just a script that will take the environment as an argument and then use the same script with the three different environments? |
.travis.yml
Outdated
| @@ -1,3 +1,5 @@ | |||
| language: node_js | |||
| node_js: | |||
| - "node" | |||
| before_script: | |||
There was a problem hiding this comment.
Thinking since we're customizing the build so much at this point we may as well override script instead of defining as before_script, then move all tasks (including default test) into a new ci npm script.
package.json
Outdated
| @@ -15,7 +15,8 @@ | |||
| "build": "cross-env BABEL_ENV=default NODE_ENV=production webpack", | |||
| "lint": "eslint .", | |||
| "dev": "cross-env BABEL_ENV=default webpack --watch", | |||
| "test": "npm run lint && npm run test-unit" | |||
| "test": "npm run lint && npm run test-unit", | |||
| "travis-builds": "concurrently \"npm run build\" \"cross-env BABEL_ENV=default webpack\" \"cross-env BABEL_ENV=default NODE_ENV=test webpack\"" | |||
There was a problem hiding this comment.
I don't know that we need to build all the environments. I think this could be simplified to:
"ci": "concurrently \"npm run build\" \"npm test\""(Assuming script instead of before_script per my previous suggestion)
Aside: How do you feel about the ci name suggestion here? I don't really have a strong preference; seemed simpler and less specific to any one CI tool in case we'd ever change.
Can we also fix the whitespace here to use spaces instead of tabs? Spaces are an exception for this file because the default behavior of npm install --save (and --save-dev) will use spaces anyways. We define an EditorConfig configuration for the project which enforces this (source). Installing a plugin for your editor will avoid the need for you to account for this manually.
There was a problem hiding this comment.
Can't use npm test persee only test env as test has the --watch flag which will not error properly to fail the CI build.
There was a problem hiding this comment.
Wait nvm confusing that with dev.
There was a problem hiding this comment.
Good catch on the tab as well :). Those sound like good changes.
|
@aduth Should be good now. |
Fixes #481. Adds the build step to Travis CI. Since we are using
webpack v2, the build will exit with a non zero error code, if any build
failures occur.