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.

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

include nvm and travis ci config, build with node 8/npm 5 #25

Merged
merged 1 commit into from
Sep 22, 2017

Conversation

bcruddy
Copy link
Contributor

@bcruddy bcruddy commented Sep 21, 2017

In reference to #24.

This includes:

  • .nvmrc to point at node 8
  • package-lock.json as a part of npm 5
  • travis config

I'd be happy to swap out node versions or another CI. I also obviously cannot set up the hooks for travis to run in this repo so if we do go the travis route that will need to be added.

@stasm
Copy link
Owner

stasm commented Sep 21, 2017

@bcruddy Nice, I love this. Especially the CI part :) Do I need to enable Travis before and after this is merged?

@bcruddy
Copy link
Contributor Author

bcruddy commented Sep 21, 2017

Yeah if you enable the travis hook we can make sure everything runs as expected (it should run if I force push to the branch) before it gets merged

@stasm
Copy link
Owner

stasm commented Sep 21, 2017

Alright, I've enabled the Travis hook for this repo. Do you want to try it out?

@@ -0,0 +1 @@
v8
Copy link
Owner

Choose a reason for hiding this comment

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

Is this file specifically for Travis or for using with nvm use? Also, should we also add this version to package.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it's not but Travis does use nvm to set the node version. I tend to work with different node versions across different projects and it's always nice to guarantee I'm using the correct version or switch to it with a simple terminal command. The engine version in package.json should be the minimum version supported.

@stasm
Copy link
Owner

stasm commented Sep 21, 2017

I was able to test the integration by manually triggering a build within the Travis UI: https://travis-ci.org/stasm/innerself/builds/278360298

@stasm stasm merged commit eac8cb9 into stasm:master Sep 22, 2017
@bcruddy bcruddy deleted the feature-ci-nvm branch September 22, 2017 13:49
@bcruddy
Copy link
Contributor Author

bcruddy commented Sep 22, 2017

@stasm I'm not sure if its just not showing up until someone pushes on an open PR but there's a repository settings to force CI to pass before merging (it can be overridden by admins)

@bcruddy bcruddy mentioned this pull request Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants