search

dalen / puppet-puppetexplorer

Puppet module for installing Puppet Explorer
29 stars 21 forks source link

add nginx support #13

Closed bastelfreak closed 8 years ago

bastelfreak commented 9 years ago

did a little rewrite and added the params.pp design pattern. also added a part for nginx. working fine on centos7

dalen commented 9 years ago

This change introduces some lint errors I think it would be good to fix. Also I think it would make sense to use the jfryman-nginx module as that seems to be the most popular community module for managing nginx instead of just using a file resource.

bastelfreak commented 9 years ago

I fixed a few errors from puppet-lint, but don't know why it blames "manifests/nginx.pp:26:arrow_alignment:WARNING:indentation of => is not properly aligned". In my opinion the alignment is fine.

I know that the nginx template is hackish, but it works fine. I wasn't able to produce a suitable vhost with the jfryman-nginx module

dalen commented 9 years ago

I think it complains because there are two spaces before the => instead of just one. The error message for that should really be improved in puppet-lint though :)

Hmm, what part was tricky with the nginx module? I haven't really used it that much myself, perhaps @daenney can help out?

bastelfreak commented 9 years ago

stupid error messages... I use tabs to align the code, but they get expanded to spaces. otherwise code would be ugly if several developer are working on it and some use tabstop=2 and others tabstop=4

daenney commented 9 years ago

Yeah, the error is the double space between ensure and the => and the subsequent ones.

daenney commented 9 years ago

I'm inclined to agree with @dalen though. For nginx, use jfryman/nginx.

This is also a way too opinionated template, people might want their logs in different places, other upstream naming conventions, different ciphers and weaker protocols because of platforms and so on and so on. Unless you want to make all that configurable, I don't think this is a good idea.