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

Use bravado-asyncio as default HTTP client #2

Merged
merged 1 commit into from Jan 30, 2018
Merged

Conversation

sjaensch
Copy link
Owner

This removes the duplicate implementation; I'm also doing some minor documentation fixes.

@sjaensch sjaensch requested a review from cklein January 29, 2018 10:58
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 96.287% when pulling dfacf26 on use-bravado-asyncio into 2cce48f on master.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 29, 2018

Coverage Status

Coverage decreased (-1.2%) to 96.287% when pulling dfacf26 on use-bravado-asyncio into 2cce48f on master.

Copy link
Collaborator

@macisamuele macisamuele left a comment

Choose a reason for hiding this comment

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

Really nice change 👍
🚢

@macisamuele
Copy link
Collaborator

Checking coverage data I see that no tests touch http_client.py.
I would suggest to add tests for that or to nuke it

@sjaensch
Copy link
Owner Author

I'd nuke it if we remove all code that we can just reuse from bravado. If I delete it it will just cause merge conflicts if the file ever changes upstream. Also it keeps the promise that in your code you can move from bravado to aiobravado, without having to know which modules are where.

@sjaensch sjaensch merged commit 77f41c6 into master Jan 30, 2018
@sjaensch sjaensch deleted the use-bravado-asyncio branch January 30, 2018 12:25
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

3 participants