david22swan
david22swan
Ah, this is just a blanket PR to add to all the metadata's The actual change's will be added on an on need basis as I go through them all
@BDelacour Hey, sorry it's taken so long to comment on this Anyway the change look's ok to me, but would prefer it to have a more descriptive name, i.e. `vhost_enabled`,...
@cmusik Look's like you've got a bunch of syntax errors. The only test failure I see looks to be transient though.
@cmusik While this looks good to me on a first glance I would prefer that it have at least one test to cover the new functionality.
@raybellis Unsure if this is needed, was under the impressions that `serveradmin` is supposed to be passed through to _file_header.erb and set there?
@raybellis Any chance of you coming back to this?
Not sure what you mean by infrastructure, by adding testing I was referring to the addition of a small test such as this: https://github.com/puppetlabs/puppetlabs-apache/blob/main/spec/acceptance/apache_parameters_spec.rb#L323 Something to show that the parameter...
@raybellis ^^
@ekohl Apologies for the late review. Anyway this look's like a good change to me but was wondering if you had more work that you intended to add to it...
@ekohl Look's like your getting some failures across the Redhat OSs Though they don't look fully consistent, some variance across the failures