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

Handle network requests in Airrecord::Client #54

Closed
wants to merge 4 commits into from
Closed

Conversation

chrisfrank
Copy link
Collaborator

Fixes #44.

While I was at it, I thought it would be helpful to draw firmer boundaries between Table and Client. Specifically, Table no longer manipulates network requests directly — it just asks Client to make requests, and lets Client handle escaping, parsing, errors, etc.

I think this will make it easier to add features to Table, like finder methods (#19), validations, etc. But I may have gotten a bit carried away.

@@ -32,22 +32,79 @@ def connection
}
end

# TODO: remove in v2
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These methods were all undocumented and probably only ever called internally. But I guess we should wait until v2 to remove them, in case anyone has been calling Airrecord::Client.connection.handle_error directly.

@chrisfrank
Copy link
Collaborator Author

This PR turned out to be not at all helpful in adding finder methods (#56), and I think was trying to solve a non-existent problem. Shall I go ahead and close it @sirupsen?

@Meekohi
Copy link
Collaborator

Meekohi commented Jan 8, 2021

I'd +1 closing this unless anyone wants to revive it.

@sirupsen
Copy link
Owner

sirupsen commented Jan 9, 2021

👍 Feel free to close

@Meekohi Meekohi closed this Jan 9, 2021
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.

Airrecord::Client's connection has no adapter
3 participants