Skip to content
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

EL7 uses conf.modules.d directory for modules. #1305

Merged
merged 1 commit into from Dec 23, 2015
Merged

EL7 uses conf.modules.d directory for modules. #1305

merged 1 commit into from Dec 23, 2015

Conversation

jasonhancock
Copy link
Contributor

This is briefly mentioned in MODULES-1734, but EL7 uses the conf.modules.d directory for module configuration.

$mod_dir = "${httpd_dir}/conf.d"
$mod_dir = $::apache::version::distrelease ? {
'7' => "${httpd_dir}/conf.modules.d",
default => "${httpd_dir}/conf.d",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be using versioncmp()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, for the sake of consistency and maintainability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to talk consistency, then look further down in the file (line 101) where it's using the same syntax I'm using.

@bmjen
Copy link
Contributor

bmjen commented Dec 22, 2015

@jasonhancock Are you able to address the feedback left by @igalic and also squash the commits?

@bmjen
Copy link
Contributor

bmjen commented Dec 22, 2015

@jasonhancock you're right. Apache in general needs some cleanup. Do you mind squashing? I'll investigate the pcci failures.

@bmjen
Copy link
Contributor

bmjen commented Dec 22, 2015

@jasonhancock it appears the acceptance test failures on centos7 are related to this change.

This is briefly mentioned in
[MODULES-1734](https://tickets.puppetlabs.com/browse/MODULES-1734), but
the EL7 uses the conf.modules.d directory for module configuration.

Also cleaned up some of the acceptance tests to use the common config in
`version.rb` to DRY
@jasonhancock
Copy link
Contributor Author

@bmjen commits squashed and my test failures fixed. I'm not sure I understand the Ubuntu 14.04 fastcgi failure. Looks like it's not finding the libapache2-mod-fastcgi package, but I was unable to repoduce the issue locally on a 14.04 vagrant VM.

@bmjen
Copy link
Contributor

bmjen commented Dec 23, 2015

@jasonhancock Thanks! Ignore the fastcgi errors those are fixed in master.

bmjen added a commit that referenced this pull request Dec 23, 2015
EL7 uses conf.modules.d directory for modules.
@bmjen bmjen merged commit 2a60859 into puppetlabs:master Dec 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants