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 URL to to_hash in Faraday::Response (#1474) #1475

Merged
merged 2 commits into from
Jan 16, 2023

Conversation

aaronstillwell
Copy link
Contributor

Description

This PR adds the url to the to_hash method of Faraday::Response as discussed in #1474

Todos

Review, commentary & discussion

Additional Notes

The url is provided to the hash as-is, which means it'll still be a URI object.

See https://github.com/aaronstillwell/faraday/blob/29ac4eec651f367d4be57985297db20f10796f9d/lib/faraday/response.rb#L65

In the spec, we create a URI object to test with: https://github.com/aaronstillwell/faraday/blob/29ac4eec651f367d4be57985297db20f10796f9d/spec/faraday/response_spec.rb#L6-L9

We currently do not explicitly test that the deserialized object has a URI object (rather than the URL as a string), but manual inspection suggests this works as I'd expect as an end user (that we'd get the URI object back and not a string).

Is there any expectation that to_hash should not contain an actual URI object?

Copy link
Member

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

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

I checked the codebase and to_hash is currently only used for serialization.
marshal_dump should have no issues supporting a URI object, and if this is turned into json the URI will automatically be converted into a string.

@iMacTia iMacTia merged commit bf3ed11 into lostisland:main Jan 16, 2023
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