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 the ability to specify additional connection options #4

Merged

Conversation

jhanggi
Copy link
Contributor

@jhanggi jhanggi commented Dec 12, 2017

For example, if you need to specify ssl: { verify: false } in order to get
around a cerificate problem (which I don't recommend, but might be a stopgap),
you may specify when configuring:

config.connection_options = { ssl: { verify: false } }

This will accept any of faraday's options. See https://github.com/lostisland/faraday/blob/master/lib/faraday/connection.rb

I didn't see a changelog, so I wasn't sure how you wanted to handle what might be a breaking change for some people. In theory, it shouldn't break anything--and should just make everyone a little more secure--because the certificates should be valid, but there's a chance there's some certificate out there that isn't.

Also, I looked but didn't see any examples of your preferred hash syntax, so if it needs to support older =>, that's an easy change. And I'm not fluent in minitest (I spend most of my life in rspec), so if there's a preferred idiom, I'm happy to change it.

Addresses Issue #3.

For example, if you need to specify `ssl: { verify: false }` in order to get
around a cerificate problem (which I don't recommend, but might be a stopgap),
you may specify when configuring:

```ruby
config.connection_options = { ssl: { verify: false } }
```

This will accept any of faraday's options. See https://github.com/lostisland/faraday/blob/master/lib/faraday/connection.rb
@thetizzo
Copy link
Contributor

This looks good, thanks! I will get this merged and push out a release sometime soon.

Also changelog is a great idea. I will add that and attempt to add the changes from previous releases as much as I can as well as some notes for this change. Otherwise I was trying to use semantic versioning as a reasonable approximation of when there might be a breaking change for users.

As far as hash syntax goes, we don't have any requirements for that since we only support back to ruby 1.9.3 which has both hash syntaxes anyway.

@thetizzo thetizzo merged commit 2c25d8a into mobilecause:master Dec 13, 2017
@thetizzo
Copy link
Contributor

@jhanggi just released this change as v2.1.0. Thanks for the PR! 🎉 🎉 🎉

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.

None yet

2 participants