-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
bpo-36932: use proper deprecation-removed directive #13349
Conversation
.. And update some deprecation warnings with version numbers.
11d99bc
to
c944302
Compare
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.
Nice, always good to see some more structure and detail added to these things.
I'd love to be able to build an index of deprecated/removal of this is used
consistently. Though not want to write a Sphinx extension...
…On Wed, May 15, 2019, 19:28 Paul Ganssle ***@***.***> wrote:
***@***.**** approved this pull request.
Nice, always good to see some more structure and detail added to these
things.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#13349?email_source=notifications&email_token=AACR5T2D6TFN7YNG6SNRDK3PVTBGLA5CNFSM4HNHR2KKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBYY65MQ#pullrequestreview-238153394>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACR5TZGSOCDTMKBM2AXGTLPVTBGLANCNFSM4HNHR2KA>
.
|
@@ -1353,7 +1353,7 @@ always available. | |||
This function has been added on a provisional basis (see :pep:`411` | |||
for details.) Use it only for debugging purposes. | |||
|
|||
.. deprecated:: 3.7 | |||
.. deprecated-removed:: 3.7 3.8 |
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.
Side question:
Do we really want to remove it in 3.8?
If yes -- the time window is very close.
@1st1 we need your judgment for it.
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.
I think we should drop it. asyncio no longer needs it, AFAIK Trio considered using it but decided it's too slow.
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.
I can do it myself tomorrow.
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.
Nice!
Thank you very much!
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.
@1st1 I've opened https://bugs.python.org/issue36933 yesterday to track the removal.
Doc/library/asyncio-task.rst
Outdated
in Python 3.10. | ||
.. deprecated-removed:: 3.8 3.10 | ||
|
||
The *loop* argument is deprecated and scheduled for removal |
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 text should be updated.
Now it looks like:
Deprecated since version 3.8, will be removed in version 3.10: The loop argument is deprecated and scheduled for removal in Python 3.10
Looks like battology.
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.
I spotted that and didn't wanted to change too much at once, but sure I can take care of it.
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.
Please fix deprecation texts to remove generated wordiness
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Thanks @asvetlov I have made the requested changes; please review again |
Thanks for making the requested changes! @asvetlov: please review the changes made to this pull request. |
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!
Sorry, I can't merge this PR. Reason: |
Thanks @Carreau for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
Sorry, @Carreau, I could not cleanly backport this to |
Well, if doc change cannot be backported to 3.7 automatically I prefer to keep it in 3.8 only. Thanks, @Carreau ! |
Ok, let me know I can work on backporting it otherwise. |
Backporting would be good but I don't insist at all. |
Well, I prefer to focus on features then, (like top-level-await #13148) before beta1, once we are in the beta period I can work on applying that to 3.7. |
.. And update some deprecation warnings with version numbers.
https://bugs.python.org/issue36932