search

plathrop / puppet-module-supervisor

Puppet module for configuring the supervisor daemon tool.
BSD 3-Clause "New" or "Revised" License
6 stars 0 forks source link

Include class supervisor in class supervisor::service #24

Closed saz closed 11 years ago

saz commented 11 years ago

@plathrop It would be great, if you can give this change a try, before I'll merge this.

The supervisor::update class is required, to avoid a dependency cycle.

saz commented 11 years ago

If this is merged, I'll fix #23

plathrop commented 11 years ago

I'd love to try it but I don't have anywhere using this code right now :-(

The original thinking behind not doing the include in the service define was that someone might want to manage the base supervisor stuff with a subclass of their own. I don't know if anyone is relying on this, and it may have been a case of premature optimization, but it was by design, not by accident. Again, since you are actually using this stuff, I'll bow to your judgement on this if you end up disagreeing with the logic.

I'm not sure I see how this change relates to #23, though...

saz commented 11 years ago

This change will make the module easier to use, without adding a constraint such as require => Class['supervisor'] on every supervisor service. If those are omitted, it's possible, that dependencies are in the wrong order.

This change has nothing to do with #23, except that I was to lazy to add a separate branch :)

plathrop commented 11 years ago

Seems sensible enough to me. Go for it.

plathrop commented 11 years ago

Or as I always say at work: :ship: it.