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 use python2-pip package name #349

Closed
wants to merge 1 commit into from

Conversation

david-caro
Copy link

It turns out that the package name has changed in the epel repository.
closes #348

Signed-off-by: David Caro [email protected]

It turns out that the package name has changed in the epel repository.
closes voxpupuli#348

Signed-off-by: David Caro <[email protected]>
@schwicke
Copy link

This certainly would do the job. I wonder though if it would not be a good idea to allow the user to pass on the package name as a parameter. This way, it may be possible to simplify the code a bit.

@david-caro
Copy link
Author

david-caro commented Dec 12, 2016

I agree that is a good idea, and that it would have allowed me to work around the issue, though to me that should be the last resource for the user, it should pick the good package by default.

@ghoneycutt
Copy link
Member

Thank you for your contribution! Could you please add a spec test so that we can merge this change.

@ghoneycutt
Copy link
Member

@schwicke The package name should definitely be a parameter, though we also want to have sane defaults for our supported platforms so that when you use the module, it does the right thing out of the box without having to specify data.

@ju5t
Copy link
Contributor

ju5t commented Dec 12, 2016

You probably missed #346 when writing this pull request. Feel free to reuse the test I wrote.

I would recommend versioncmp() over a regex on the wrong fact. You can use $::operatingsystemmajrelease instead.

@ghoneycutt
Copy link
Member

Hi,

Could someone please do a functional test of the code from #347 and verify that it works to solve this problem?

@ghoneycutt
Copy link
Member

@david-caro Thanks for the contribution though going with PR #347 since it had spec tests. We could still use your help by verifying that the code in PR #347 works.

@ghoneycutt ghoneycutt closed this Dec 12, 2016
@david-caro
Copy link
Author

@ghoneycutt I did not see it, I think I searched on the issues but not the PRs. No problem 👍

@schwicke
Copy link

@ghoneycutt fully agree, I think the module should both provide sensible (working) defaults and the option to name the package as a parameter. That would be nice to have.

@david-caro david-caro deleted the fix_el7_package_name branch December 13, 2016 09:36
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.

python-pip has been renamed to python2-pip on el7 epel repo
4 participants