-
Notifications
You must be signed in to change notification settings - Fork 87
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
refs #11280 - drop Ruby 1.8 support #176
Conversation
[test] |
We're still a bit limited on what we can update with regards to RPM installations, since some dependencies are shared by kafo and the smart proxy, which still support 1.8.7. We don't have the means to build multiple versions of one package (I'm unsure if this is technically possible with our packaging repo, needs investigation). This would affect logging, highline and rest-client among others. It'd be fine to lift the maximum version in hammer-cli itself, provided the minimum isn't raised! We'd need to ensure logging 1.x, highline 1.6 and rest-client 1.6 etc also work. |
Yes, at least with highline I'm very sure that it works with <1.7 on Ruby 1.8 and 1.7.x on higher Ruby versions. With logging I'm unsure, as said and with rest-client the problem is more related to the verify_ssl thing which I want to tackle seperately. In general I can say we'll run into a lot of problems when we will start packaging for Ubuntu 16.04, as quite some gem packages already where updated in upstream Debian/Ubuntu now. |
I think for logging it was only a small incompatibility that we hit in Foreman core, and decided to support both for the same reasons: https://github.com/theforeman/foreman/blob/develop/lib/foreman/logging.rb#L96-L101 But yep, fine by me to raise maximums wherever possible. |
I did some testing and it works ok. So fine by me to merge that. @mbacovsky ? |
👍 While rest-client dependency will be fixed in separate PR this looks good to merge. |
Merging, thanks @mmoll |
refs #11280 - drop Ruby 1.8 support
I did leave rest-client untouched for now, as 1.7 does verify_ssl by default.
I'm unsure about the major version bump of logging, but at least an error got logged as expected with 2.0.0 in my tests.
Discuss! 💥