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

Speedy try at letting command/cb functions be async/awaitable #128

Closed
wants to merge 2 commits into from

Conversation

jessekrubin
Copy link

Seems to work.
Modified one of your (@tiangolo) test files.

The only thing that is funky is the click 2-3 param warning I get when doing something with context.

@codecov
Copy link

codecov bot commented Jun 26, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f5682d3) to head (a4116e1).
Report is 252 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##            master      #128    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          242       243     +1     
  Lines         4502      4634   +132     
==========================================
+ Hits          4502      4634   +132     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@thedrow thedrow left a comment

Choose a reason for hiding this comment

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

Please refrain from including code formatting in this PR.
It's very hard to review the changes this way.

@jessekrubin
Copy link
Author

Please refrain from including code formatting in this PR.

It's very hard to review the changes this way.

@thedrow I used the script for formatting. I think my wsl git changed the file permissions which makes git think the whole of a file(s) was/were changed

@hozn
Copy link

hozn commented Mar 24, 2022

Is this still on the slate for inclusion or is this OBE?

@svlandeg svlandeg added feature New feature, enhancement or request p3 labels Mar 1, 2024
@svlandeg
Copy link
Collaborator

svlandeg commented Mar 8, 2024

Closing this PR in favour of #213 which contains a more clean version of the diff. Thanks for your contribution @jessekrubin and apologies for taking so long to engage with this! We'll continue the review over at #213.

@svlandeg svlandeg closed this Mar 8, 2024
@jessekrubin
Copy link
Author

@svlandeg no prob. The formatting got all screwy (I think bc I was on a wonky computer).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature, enhancement or request p3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants