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

Add EPEL dependency #198

Merged
merged 1 commit into from
Jun 13, 2015
Merged

Conversation

gene1wood
Copy link
Contributor

This approach to adding the EPEL dependency uses a few more lines of code than the alternative, but it means that the EPEL module only gets called if the OS (RedHat) and OS Version require it.

Note: Even if the EPEL module were included on a different OS (e.g. Ubuntu) the module logic of the EPEL module doesn't do anything unless it's on a RedHat osfamily system.

An alternative method to accomplish this would be to just unconditionally include the EPEL module, and set it as a require for the pip and virtualenv package resources. On Ubuntu this would mean that puppet output would show the EPEL module being used but nothing would be done to the system. On RedHat the EPEL yum repo configs would be installed even if the user was on RHEL 7 and only looking to install virtualenv (not pip) in which case EPEL is not required because RHEL 7 provides virtualenv in the base repositories.

I figured that the method in this PR would be better than the alternative as it would only involve the EPEL module when required, even though that resulted in a couple more lines of conditional logic.

One other thing to note is that regardless of the OS (Ubuntu RedHat), the EPEL module would be downloaded because this PR sets it as a metadata.json dependency. This means on an Ubuntu system, puppet (or librarian-puppet or whatever you use to fetch puppet modules) would download the EPEL module, but would not use it. It would just be in the modules directory unused. I can't think of a way around this because module dependencies (like those recorded in metadata.json) aren't conditional and can't depend on the OS. I don't think it's a big problem though.

Fixes #196

@gene1wood
Copy link
Contributor Author

This PR may also address #115

shivapoudel added a commit that referenced this pull request Jun 13, 2015
@shivapoudel shivapoudel merged commit 3392b4f into voxpupuli:master Jun 13, 2015
@gene1wood gene1wood mentioned this pull request Jun 13, 2015
@gene1wood gene1wood deleted the add-epel-dependency branch July 21, 2015 16:03
shivapoudel pushed a commit that referenced this pull request Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants