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

Require rest-client 2.x, drop 1.x support code #269

Merged
merged 2 commits into from
Oct 11, 2017

Conversation

cben
Copy link
Collaborator

@cben cben commented Sep 18, 2017

Closes #264.

Made it ~> 2.0 — will allow any 2.y.z; I don't want to pin to 2.0.z without specific reasons as rest-client is widely used gem (and my impression is they're careful about compatibility so we can trust their semver)...
Tests pass locally against:

  • 2.0.0
  • 2.0.2 (current)
  • 2.1.0rc1

@cben cben mentioned this pull request Sep 18, 2017
@simon3z
Copy link
Collaborator

simon3z commented Sep 18, 2017

+1 on the effort, haven't reviewed in detail.
@moolitayer PTAL

@@ -441,18 +441,6 @@ def api
JSON.parse(response)
end

def self.restclient_read_timeout_option
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 🎉

@@ -27,7 +27,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'webmock', '~> 3.0.1'
spec.add_development_dependency 'vcr'
spec.add_development_dependency 'rubocop', '= 0.49.1'
spec.add_dependency 'rest-client'
spec.add_dependency 'rest-client', '>= 2.0.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed I'm fine with both ~> 2.0.0 or ~> 2.0 here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

>= 2.0.0 was wrong as it would also accept 3.0.
Made it ~> 2.0 — will allow any 2.y.z; I don't want to pin to 2.0.z without specific reasons as rest-client is widely used gem...
Tests pass locally against:

  • 2.0.0
  • 2.0.2 (current)
  • 2.1.0rc1

and rest-client is generally careful about backward compat so I think we can trust their semver.

I can make travis test against rest-client master in parallel to 2.0, if anyone wants.

Copy link
Collaborator

@moolitayer moolitayer left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@simon3z simon3z merged commit 2700c33 into ManageIQ:master Oct 11, 2017
@cben cben added the v2.x/no label Jan 21, 2018
cben added a commit to cben/kubeclient that referenced this pull request Jan 25, 2018
Backporting ManageIQ#223 bumped to rubocop with different defaults.
These rubocop errors are fixed on master by ManageIQ#253 and ManageIQ#269
but we didn't backport those.
cben added a commit to cben/kubeclient that referenced this pull request Jan 25, 2018
Backporting ManageIQ#223 bumped to rubocop with different defaults.
These rubocop errors are fixed on master by ManageIQ#253 and ManageIQ#269
but we didn't backport those.
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.

3 participants