david22swan

Results 56 comments of 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...

@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