search

netmanagers / puppet-nginx

[UNMAINTAINED] Puppet Nginx module
http://www.example42.com
Other
37 stars 60 forks source link

Added a few features #35

Closed mburger closed 11 years ago

mburger commented 11 years ago
alvagante commented 11 years ago

The additions make sense at first sight, but would like to involve other committers on this, as I have not real environments where to test them. @erik-smit @javierbertoli @pafei @tiengo @DavidS ... what do you think about this PR? Is it safe to merge it? I expect at least a service restart, since something changes in the config files, but that might be acceptable.

javierbertoli commented 11 years ago

They look OK to me. My only concern would be that the concat fragment file names are modified (ie, in vhost.pp

concat::fragment { "${name}+69.tmp":

changes to

concat::fragment { "${name}+00.tmp":

and that would generate new files without deleting/overwriting the old ones in an existing setup. Am I right about that?

mburger commented 11 years ago

the diff view is a little misleading here because i moved some code around :-/ the name of the fragments shouldn't change

alvagante commented 11 years ago

Had to manually merge this one, would you please verify if everything is ok? Also it would be very useful to have soem same usage code for this (and others) module, that use the various nginx::resource defines. Please feel free to make a PR for manifests in the test/ directory that can be easily tested on a live system (they would be useful also for rspec-system tests)

mburger commented 11 years ago

Thank's for the merge, while testing this i found a overlooked typo in the provided template conf.d/nginx.conf.erb and added proper support for debian. As for the Tests, we have a Testsuite based on Rouster https://github.com/chorankates/rouster i'll see if we can convert it to rspec-system

i'll open another merge request for that one :)