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 #1876 - remove npm update #1877

Closed
wants to merge 1 commit into from

Conversation

@Mte90
Copy link
Contributor

commented Jun 22, 2019

Now npm is handled by nvm so we don't need any checks for it anymore

fix #1876 - remove npm update
Now npm is handled by nvm so we don't need any checks for it anymore

@Mte90 Mte90 requested review from tomjn and Varying-Vagrant-Vagrants/testers Jun 22, 2019

@tomjn

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

I thought we were removing nvm now that we've switched back to Node 10?

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

We are still using it so we need to choose what to do, right now npm is symlinked to the nvm version.
Or we remove nvm everywhere or we change that behaviour.

@tomjn

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

We are still using it so we need to choose what to do, right now npm is symlinked to the nvm version. Or we remove nvm everywhere or we change that behaviour.

There is no NVM in VVV core, the only use is in the custom-site-template-develop, which is the only place it's used.

It was added because we installed Node 11, and core needs Node 10. This has since been fixed in VVV core, so the NVM change is now no longer needed. We should remove it

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

So you mean a pr to remove https://github.com/Varying-Vagrant-Vagrants/custom-site-template-develop/blob/master/provision/vvv-init.sh#L23-29 this lines?
I can do it asap and close this one but we need to add something that remove nvm (or we suggest a vagrant destroy on updating).

GitHub
For working with WP Core development. Contribute to Varying-Vagrant-Vagrants/custom-site-template-develop development by creating an account on GitHub.
@tomjn

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

I can do it asap and close this one but we need to add something that remove nvm (or we suggest a vagrant destroy on updating).

I'm happy for commands that do that be added to the core provisioner once nvm is removed from custom site template develop

nvm-sh/nvm#298

@Mte90 Mte90 closed this Jun 27, 2019

@Mte90 Mte90 deleted the mte90/npm-update-removed branch Jun 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.