-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-24755: Document asyncio.wrap_future #603
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
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
@adisbladis, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1st1, @asvetlov, @bitdancer, @berkerpeksag and @eliben to be potential reviewers. |
Doc/library/asyncio-task.rst
Outdated
@@ -540,6 +540,13 @@ Task functions | |||
|
|||
.. deprecated:: 3.4.4 | |||
|
|||
.. function:: wrap_future(future, \*, loop=None) | |||
|
|||
Wrap a :class:`concurrent.futures.Future` object in a :class:`Future` object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing periods at the end of sentences. Make sure that max line length is 72.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the input. I have made a new commit and rebased.
CLA has been signed. |
Doc/library/asyncio-task.rst
Outdated
Wrap a :class:`concurrent.futures.Future` object in a | ||
:class:`Future` object. | ||
|
||
When the original :class:`concurrent.futures.Future` is completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence sounds odd to me.
What about...
The wrapping of :class:`Future` ends when the original :class:`concurrent.futures.Future` is completed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wrapping doesn't end, though. It's that completion of the wrapped future triggers completion of the wrapper. Which is what I would expect from a wrapper...and I would likewise expect that result on the wrapper will return the result of the wrapped future, so I'm not even sure this sentence is a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Perhaps just remove this sentence then.
@adisbladis Could you remove the sentence? Thanks.
Doc/library/asyncio-task.rst
Outdated
Wrap a :class:`concurrent.futures.Future` object in a | ||
:class:`Future` object. | ||
|
||
When the original :class:`concurrent.futures.Future` is completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Perhaps just remove this sentence then.
@adisbladis Could you remove the sentence? Thanks.
(cherry picked from commit 824f687)
(cherry picked from commit 824f687)
Thanks for the PR @adisbladis 🎉 |
No description provided.