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.

Sign up for GitHub

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

Jump to bottom

Changes the way adapters are managed #750

Merged
merged 2 commits into from
Nov 21, 2017
Merged

Changes the way adapters are managed #750

merged 2 commits into from
Nov 21, 2017

Conversation

iMacTia
Copy link
Member

@iMacTia iMacTia commented Nov 19, 2017

Description

Changes the way adapters are managed to gain the following:

  • Adapter can be set anywhere in the stack and will always be the last
  • We can easily check if the adapter is set or not
  • Use default adapter if no adapter set

Fixes #47 and #121

@iMacTia
Changes the way adapters are managed to gain the following:
eec41ab 
* Adapter can be set anywhere in the stack and will always be the last
* We can easily check if the adapter is set or not
* Use default adapter if no adapter set

fixes #47
fixes #121
@iMacTia
Copy link
Member Author

iMacTia commented Nov 19, 2017

@olleolleolle want to have a look 😄 ?

olleolleolle
olleolleolle reviewed Nov 19, 2017
View reviewed changes
lib/faraday/connection.rb

if handler
handler.klass.setup_parallel_manager
if adapter and adapter.respond_to?(:supports_parallel?) and adapter.supports_parallel?
Copy link
Member

Choose a reason for hiding this comment

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

You may wish to use && over and?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that's the Rubocop convention and I've already planned to scan the entire gem through it before releasing 1.0. However in that file and is preferred to && so I went for that.
It's just temporary, we'll eventually converge into the standard and fix all these occurrences later on

olleolleolle
olleolleolle approved these changes Nov 19, 2017
View reviewed changes
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 makes very happy.

@iMacTia iMacTia merged commit 8503bae into 1.0 Nov 21, 2017
@iMacTia iMacTia deleted the feature/1.0/new-adapters-handling branch November 21, 2017 09:48
@iMacTia iMacTia added this to the v1.0 milestone Nov 22, 2017
@olleolleolle olleolleolle mentioned this pull request Dec 25, 2017
Closed
@dentarg dentarg mentioned this pull request Jan 8, 2018
Closed
@d-m-u d-m-u mentioned this pull request Feb 6, 2020
Merged
@buty4649 buty4649 mentioned this pull request Mar 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olleolleolle olleolleolle olleolleolle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@iMacTia @olleolleolle