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

Build on install #73

Merged
merged 4 commits into from
Mar 1, 2018
Merged

Build on install #73

merged 4 commits into from
Mar 1, 2018

Conversation

elhoyos
Copy link
Contributor

@elhoyos elhoyos commented Feb 27, 2018

I'm having trouble to npm install on a project using a fork of unleash-client-node because package.json is explicitly stating to never build on install.

#15 seems to be the reason behind it, but filesystem install tests suggests we don't need it anymore.

@coveralls
Copy link

coveralls commented Feb 27, 2018

Coverage Status

Coverage remained the same at 98.226% when pulling 974e325 on elhoyos:build-on-install into b373de6 on Unleash:master.

@ivarconr
Copy link
Member

@SimenB any objections?

@SimenB
Copy link
Collaborator

SimenB commented Feb 28, 2018

I don't think so - I can't remember the issue.

@elhoyos
Copy link
Contributor Author

elhoyos commented Feb 28, 2018

Please notice that despite this change works for a working directory installation for both npm install and yarn, it does not work for yarn when used as a github repository.

$ yarn add elhoyos/unleash-client-node#4a85ba6
$ ls -l node_modules/unleash-client/lib # empty
$ rm -rf node_modules
$ yarn
$ ls -l node_modules/unleash-client/lib # empty

This is a yarn issue: yarnpkg/yarn#5235

@ivarconr ivarconr merged commit 2af9cee into Unleash:master Mar 1, 2018
@elhoyos elhoyos deleted the build-on-install branch March 2, 2018 15:12
@elhoyos elhoyos restored the build-on-install branch March 2, 2018 15:30
@elhoyos elhoyos mentioned this pull request Mar 2, 2018
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.

4 participants