search

ajcrowe / puppet-supervisord

Puppet Module to install and configure applications under supervisord
MIT License
37 stars 104 forks source link

Params should use main class parameters #64

Closed willaerk closed 9 years ago

willaerk commented 9 years ago

If you override the supervisord::executable_path parameter in the main class, it should affect the supervisord::executable and supervisord::executable_ctl parameters.

Without this, the value of both executables is calculated locally in params.pp, without taking into account the main class parameter.

With the patch, the value of supervisord::executable_path is fetched from the main class. If the main class does not override the parameter, there is a fallback to the params class, resulting in the current behaviour.

willaerk commented 9 years ago

Hmmm,

travis build fails for unrelated reason: "Failed to clone git repository"

ajcrowe commented 9 years ago

Thinking about it, I'm not sure the $executable_path should really be exposed. might be better to just let people override $executable and $executable_ctl?

willaerk commented 9 years ago

Yeah, overriding $executable and $executabl_ctl is what I'm doing right now. So I'm all for removing the $executable_path parameter. The reason for the implementation in the pull request is compatibility. Removing a parameter is an incompatible, breaking change. But I agree it is the better option.

I can amend the pull request in that way if you want, just let me know.

ajcrowe commented 9 years ago

Yeah, I think given that setting $executable_path doesn't actually have any impact we can probably remove it with a note in the change log for 0.6.

If you amend the pull to simple remove this option from init.pp that would probably be best.