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

(fix): Use npm to install deps in ci. #153

Merged
merged 4 commits into from
May 2, 2018
Merged

(fix): Use npm to install deps in ci. #153

merged 4 commits into from
May 2, 2018

Conversation

olizilla
Copy link
Collaborator

@olizilla olizilla commented May 1, 2018

Updates Makefile to use npm@6 and the npm ci command, and updates the package-lock.json.

Local testing showed that the yarn install and build left the production bundle firing off errors, while the npm build doesn't. I don't know why. Details are in #150

Adds documentation on how the deploy happens for future code spelunkers.

Fix #150

@olizilla olizilla requested a review from victorb May 1, 2018 22:33
@olizilla
Copy link
Collaborator Author

olizilla commented May 1, 2018

Hmm. Installing npm@6 fails in the Jenkins build. Will investigate tomorrow.

@victorb
Copy link
Contributor

victorb commented May 2, 2018

FYI: @magik6k added a feature to our pipeline so you can decide which branch gets deployed to which environment. Might be a good idea to have for peerpad: https://github.com/ipfs/jenkins-libs#example-with-multiple-branchesrecords

@olizilla
Copy link
Collaborator Author

olizilla commented May 2, 2018

@victorbjelkholm do I just need to update the Jenkinsfile to get that to work, or do I need to update anything else?

@olizilla
Copy link
Collaborator Author

olizilla commented May 2, 2018

@victorbjelkholm @diasdavid any objections to merging this? I don't now why a yarn build causes the errors seen in #150 but it's reproducible and switching to npm fixes it. Merging this PR will (should) get peerpad.net working again.

@victorb
Copy link
Contributor

victorb commented May 2, 2018

do I just need to update the Jenkinsfile to get that to work, or do I need to update anything else?

Just update ci/Jenkinsfile to have what the example I linked above has and you should be good to go!

any objections to merging this?

Nope, LGTM!

@olizilla olizilla merged commit 44f96f2 into master May 2, 2018
@olizilla olizilla deleted the fix/ci-npm-build branch May 2, 2018 18:52
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