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 status predicate methods to Response#respond_to? #482

Merged

Conversation

akamike
Copy link

@akamike akamike commented Jun 21, 2016

Expands the existing respond_to? first-party method list to include the HTTP status predicates. Useful for things like RSpec, which check respond_to? when using predicate matchers - beyond that it makes for a more consistent and predictable object (If I can call it, it should respond_to?).

Because Response inherits from BasicObject we miss out on quite a few of these pre-made tools from Object, though it looked like moving away from BasicObject was a bigger task and would require a bit more debate. I figured this was the simplest change to make.

I have not checked for other methods that could be added to RESPOND_TO_METHODS, my concern was with the predicate methods.

Expands the existing respond_to? first-party method list to include the HTTP status predicates. Useful for things like rspec, which check respond_to? when using predicate matchers.
@@ -107,6 +107,11 @@
expect(response.respond_to?(:parsed_response)).to be_truthy
end

it "responds to predicates" do
response = HTTParty::Response.new(@request_object, @response_object, @parsed_response)
expect(response.respond_to?(:success?)).to be_truthy
Copy link
Owner

Choose a reason for hiding this comment

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

Kind of concerned that we'll unknowingly break compatibility since only success? is tested, but I guess we can roll and see what happens.

@jnunemaker jnunemaker merged commit 0d808c7 into jnunemaker:master Jun 24, 2016
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