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

implement #options. #857

Merged
merged 1 commit into from
Feb 20, 2019
Merged

implement #options. #857

merged 1 commit into from
Feb 20, 2019

Conversation

technoweenie
Copy link
Member

This implements Faraday::Connection#options almost as expected, fixing #305.

We can't rename the current :options attr_reader right now for backwards compatibility reasons. Instead, this overloads the method to support both use cases. Merging the two risks some clashing, but I can only think of one issue: the other request helpers like #get can be called with no arguments:

conn = Faraday.new 'http://example.com'
conn.get # GET /
conn.options # oops!

You can get around this easily with conn.options(nil). I think it's worth introducing this regardless of the risk, assuming we'll rip it out when v1.0 ships.

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

This addition - with documentation - extends the reach if the user, to include a new HTTP verb.

👍

@iMacTia
Copy link
Member

iMacTia commented Feb 20, 2019

Ignoring CodeClimate issues, not sure they actually make sense in this case

@iMacTia iMacTia merged commit 5cad588 into master Feb 20, 2019
@iMacTia iMacTia deleted the conn-options branch February 20, 2019 22:02
@technoweenie technoweenie mentioned this pull request Jun 25, 2019
3 tasks
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.

3 participants