voxpupuli / puppet-nodejs

Puppet module to install nodejs and global npm packages
https://forge.puppet.com/puppet/nodejs
Apache License 2.0
113 stars 248 forks source link

apt and yum resources are not listed as dependencies #393

Closed logicminds closed 5 years ago

logicminds commented 5 years ago

Puppet 6 removed the apt resource from the core so it needs to be downloaded separately and listed as a dependency. Without these dependencies this modules does not work.

Would likely have to also add yum_repo as a dependency too.

Once the apt resource is added install can continue.

Affected Puppet, Ruby, OS and module versions/distributions

juniorsysadmin commented 5 years ago

apt and probably yum falls under the category of a soft dependency https://puppet.com/docs/puppet/6.0/style_guide.html#dependencies and I am strongly against adding them as a hard dependency as it causes all kinds of usability and maintenance issues longer term.

The README does mention puppetlabs-apt, but perhaps now with the removal of things from core, more needs to be listed there as part of a Pull Request.

dhoppe commented 5 years ago

@juniorsysadmin I agree that these modules should not be defined as hard dependency, but maybe we should add the following code to our modules, because not everyone reads the docs carefully:

if $facts['package_provider'] == 'deb' and !defined('apt') {
  fail('Please provide the module puppetlabs-apt')
} elsif $facts['package_provider'] == 'yum' and !defined('yumrepo_core') {
  fail('Please provide the module puppetlabs-yumrepo_core')
}
dhoppe commented 5 years ago

On second thought, I would love to see this at puppetlabs-stdlib, because we would not need to change our modules. But I think the list of modules would be endless:

juniorsysadmin commented 5 years ago

Issue #395 created to add docs