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

npm is required on 16.04 #242

Merged
merged 1 commit into from
Aug 24, 2016
Merged

npm is required on 16.04 #242

merged 1 commit into from
Aug 24, 2016

Conversation

gabriel403
Copy link
Contributor

On 16.04 we require npm to be installed along side nodejs for npm to be useable

@bastelfreak
Copy link
Member

Hi, thanks for the patch. Do you think you can add tests somehow for this?

@gabriel403
Copy link
Contributor Author

@bastelfreak would love to, but I'm not really sure how to, could you point me in the right direction? I know without it on 16.04 you get

Warning: Scope(Class[Nodejs::Params]): The nodejs module might not work on Ubuntu 16.04. Sensible defaults will be attempted.

and npm isn't useable

Notice: /Stage[main]/Nodejs::Install/Package[nodejs]/ensure: ensure changed 'purged' to 'present'
Error: Command npm is missing
Error: /Stage[main]/Boxes::Packages/Package[bower]/ensure: change from absent to present failed: Command npm is missing

@jackdpeterson
Copy link

Even without this change ... if I have defined my deps as follows:

class { '::nodejs':
  nodejs_dev_package_ensure => 'present',
  npm_package_ensure        => 'present',
}

package { 'babel':
  ensure   => 'present',
  provider => 'npm',
}

package { 'webpack':
  ensure   => 'present',
  provider => 'npm',
}

package { 'browserify':
  ensure   => 'present',
  provider => 'npm',
}

I do get the complaining about possibly not working on 16.04 ... but everything (including the npm command) worked fine for me. Perhaps there's something else related too with respect to the provider for the 16.04 release of ubuntu.

@gabriel403
Copy link
Contributor Author

@jackdpeterson yeh so you're forcing the installation of npm with the npm_package_ensure param, but should npm be installed by default? If not then yeh doing it your way makes more sense than the pr. Thoughts @bastelfreak ?

@juniorsysadmin
Copy link
Member

We don't have tests for defaults, just behaviour of parameters. Happy to merge this as is.

@juniorsysadmin juniorsysadmin merged commit f324048 into voxpupuli:master Aug 24, 2016
cegeka-jenkins pushed a commit to cegeka/puppet-nodejs that referenced this pull request Oct 23, 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.

4 participants