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

only async request and file writes #12

Merged
merged 17 commits into from
Feb 9, 2022
Merged

Conversation

avaldebe
Copy link
Collaborator

@avaldebe avaldebe commented Feb 3, 2022

Copy link
Owner

@JohnPaton JohnPaton left a comment

Choose a reason for hiding this comment

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

I really like this approach! I tried it out a bit and it also seems like a significant speedup, nice work! Just a few questions and kinks to iron out.

Form the other PR, could you still add the documentation update too? 🙏

Thanks for slimming this down, it made it a lot more manageable to review. Do feel free to open issues/PRs addressing the other contributions from #11, I'm happy for the help, it was just too broad for a single PR.

airbase/fetch.py Outdated Show resolved Hide resolved
airbase/fetch.py Outdated Show resolved Hide resolved
airbase/fetch.py Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/integration/test_airbase.py Outdated Show resolved Hide resolved
tests/test_fetch.py Show resolved Hide resolved
avaldebe and others added 6 commits February 7, 2022 09:11
Co-authored-by: John Paton <john@johnpaton.net>
Co-authored-by: John Paton <john@johnpaton.net>
@JohnPaton
Copy link
Owner

@avaldebe if this is ready for a re-review just request it or let me know 😊

@avaldebe
Copy link
Collaborator Author

avaldebe commented Feb 9, 2022

It is ready for re-review.
If I went too far with the docs setup, I can move it to a separate PR.

Copy link
Owner

@JohnPaton JohnPaton left a comment

Choose a reason for hiding this comment

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

This looks awesome! Thanks for contributing 🍰 ✨

I'm also going to switch the CI/CD over from Travis to GitHub actions right after this, then I'll do a new release.

@JohnPaton JohnPaton merged commit d296f4b into JohnPaton:master Feb 9, 2022
@JohnPaton
Copy link
Owner

I put a little acknowledgement to you in the README :)

@JohnPaton JohnPaton mentioned this pull request Feb 9, 2022
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.

use async request library
2 participants