example42 / puppet-php

A puppet module for php. According to Example42 NextGen spec.
Other
45 stars 62 forks source link

Fixing the references to Package['php'] with Package[::package] #102

Closed apengue closed 9 years ago

gwarf commented 8 years ago

This commit breaks the module (Package[php54] doesn't seem to be in the catalog) when setting:

php::package = 'php54'

As the pakcage install resource is still package['php'] and not package[$php::package] at https://github.com/example42/puppet-php/blob/v2.0.25/manifests/init.pp#L252

alvagante commented 8 years ago

@gwarf would you merge a PR with a content like this https://github.com/gnubila-france/puppet-php/commit/6e844ce0bf6bc6c95e397102c5bacf96d13d9203 ?

gwarf commented 8 years ago

It was a quick n dirty solution, one problem is that this resource name change will break code having dependencies on that particular resource name. It might be cleaner to revert those "fixes" on the references, as previoulsy it was possible to have a custom php package name and everything was working. Bu if you want I can submit a PR with the commit you mentioned.

alvagante commented 8 years ago

I don't know how many situations like that ay be around, and in such a case the quick solution would be to change the php string with the relevant variable. So I'd make the module internally coherent and do what you did in gnubila-france@6e844ce

gwarf commented 8 years ago

OK, I just created the PR.

alvagante commented 8 years ago

Thank you :-)