ajcrowe / puppet-supervisord

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

support for special 'syslog' stdout and stderr file string #41

Closed janbraiins closed 9 years ago

janbraiins commented 9 years ago
ajcrowe commented 9 years ago

Hi @honzik666 thanks for the PR, could you roll this change out to all the define types it affects and maybe add a test for it?

janbraiins commented 9 years ago

Yes, I will have a look at it

janbraiins commented 9 years ago

Hi Alex, sorry for the delayed fix. I have reworked the patch so that it fixes all affected defines. Would this be acceptable now? What kind of test should I add? I didn't extend your test cases, since I was not able to run any of them, I am getting the following error:

puppet apply --test --noop ./tests/program.pp

Error: Puppet::Parser::AST::Resource failed with error ArgumentError: Invalid resource type supervisord::program at /home/puppetadmin/production/modules/supervisord/tests/program.pp:35 on node puppet.pool Wrapped exception: Invalid resource type supervisord::program Error: Puppet::Parser::AST::Resource failed with error ArgumentError: Invalid resource type supervisord::program at /home/puppetadmin/production/modules/supervisord/tests/program.pp:35 on node puppet.pool

ajcrowe commented 9 years ago

@honzik666 this looks good to me, the tests are failing because of the fcgi-program class still has the old statement for setting the $stderr_logfile_path If you remove this and do and update the PR it should be good.

janbraiins commented 9 years ago

My bad, fixed/updated ;-)

ajcrowe commented 9 years ago

Great thanks for the contribution @honzik666