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

Refactor Adapter as final endpoints #846

Merged
merged 2 commits into from
Feb 19, 2019
Merged

Refactor Adapter as final endpoints #846

merged 2 commits into from
Feb 19, 2019

Conversation

iMacTia
Copy link
Member

@iMacTia iMacTia commented Feb 18, 2019

Description

Refactors Adapters:

  • They're now final endpoints.
  • They don't need to call @app.call anymore.
  • They don't inherit from Faraday::Middleware anymore.

Additional Notes

This is a sub-change from #845

@iMacTia
Refactors Adapters:
a5b1e27 
* They're now final endpoints.
* They don't need to call `@app.call` anymore.
* They don't inherit from `Faraday::Middleware` anymore.
@iMacTia iMacTia requested a review from mislav February 18, 2019 22:06
olleolleolle
olleolleolle reviewed Feb 19, 2019
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 extraction is much easier to inspect. Thank you for taking the time to split it up.

lib/faraday/dependency_loader.rb Outdated Show resolved Hide resolved
lib/faraday/middleware_registry.rb Show resolved Hide resolved
olleolleolle
olleolleolle approved these changes Feb 19, 2019
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.

LGTM!

@olleolleolle olleolleolle merged commit 6e4a2aa into 1.0 Feb 19, 2019
@olleolleolle olleolleolle deleted the feature/adapters-refactoring branch February 19, 2019 09:48
@technoweenie technoweenie mentioned this pull request Jun 25, 2019
Merged
3 tasks
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

@mislav mislav Awaiting requested review from mislav

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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