-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding mod_version module to be activated by default #2380
Conversation
apache::mod::version is a classthat may have no external impact to Forge modules. This module is declared in 176 of 580 indexed public
|
|
As a user of puppetlabs-apache I strongly support this PR as it re-adds the accidentally removed module version, which is an essential module and needs to be part of default_mods too. Just think about .htaccess files distributed with various software or software addons. But may I suggest adding the version module to default_mods.pp again too, @Q-Storm? |
|
Thanks @jnk0 I added the the module to the default_mods.pp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Mend error is unrelated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would have been nice to at least add a unit test.
| # @see https://httpd.apache.org/docs/current/mod/mod_version.html for additional documentation. | ||
| # | ||
| class apache::mod::version { | ||
| if $facts['os']['family'] == 'debian' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debian , if you want to get the casing right. Though I see it's also wrong in many other places.
| if $facts['os']['family'] == 'debian' { | ||
| warning("${module_name}: module version_module is built-in and can't be loaded") | ||
| } else { | ||
| ::apache::mod { 'version': } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The leading :: is not needed. AFAIK it never was for defines, but certainly since we dropped Puppet < 4 it's no longer needed.
re adding mod_version which was accidentally removed with the drop of "apache 2.2 Support" in #2329.