Skip to content
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.

Sign up for GitHub

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

Add build step to CI #499

Merged
merged 3 commits into from
Apr 24, 2017
+5 −1
Merged

Add build step to CI #499

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22d38ea
Add build step to CI
BE-Webdesign Apr 24, 2017
0bd2665
Making builds concurrent.
BE-Webdesign Apr 24, 2017
afee735
Condensing build process.
BE-Webdesign Apr 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
language: node_js
node_js:
- "node"
before_script:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- "npm run travis-builds"
4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@BE-Webdesign BE-Webdesign Apr 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait nvm confusing that with dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the tab as well :). Those sound like good changes.

},
"devDependencies": {
"autoprefixer": "^6.7.7",
Expand All @@ -28,6 +29,7 @@
"babel-plugin-transform-runtime": "^6.23.0",
"babel-preset-latest": "^6.24.0",
"chai": "^3.5.0",
"concurrently": "^3.4.0",
"cross-env": "^3.2.4",
"deep-freeze": "0.0.1",
"dirty-chai": "^1.2.2",
Expand Down