-
-
Notifications
You must be signed in to change notification settings - Fork 31.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-29879: Add versionadded where missing in typing.rst #784
Conversation
@cblegare, thanks for your PR! By analyzing the history of the files in this pull request, we identified @berkerpeksag, @zware and @brettcannon to be potential reviewers. |
@@ -500,6 +504,8 @@ The module defines the following classes, functions and decorators: | |||
|
|||
A generic version of :class:`collections.abc.Reversible`. | |||
|
|||
.. versionadded:: 3.5.2 |
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.
Insert one more space here in order for it to align with the text. Apart from this, LGTM.
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.
In facts, there are 3 and 4 spaces indentation in this file. What standard is expected ? I could make all the file standard
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.
According to the devguide, it is 3, but don't worry about that too much (i,e don't go changing all the file now). You mainly need to align the ..versionadded
with the preceding text so they are formatted as part of the same block.
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.
@DimitrisJim is right: please don't go through and do a mass update of indentation. If you're fixing it for the docs around where you added versionadded then that's fine, but otherwise just leave it be.
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.
@cblegare Are you sure that Reversible
is new in 3.5.2? I am almost sure it was always there. (It was slightly changed however around a year ago.)
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.
@ilevkivskyi I mentioned Reversible
in the tracker due to it's listing in what's new for Python 3.5.2. Here's the change you're talking about too. I guess that either the version needs to get dropped for Reversible
or the What's new entry changed, not sure about it though :-)
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.
Since Reversible
was in 3.5.0, the versionadded
is unnecessary and Misc/NEWS
should get fixed.
@cblegare Could you please resolve the conflict and implement the last comment by @brettcannon? |
@cblegare are you still working on this? If not, then I can overtake this. |
I think it's still worth updating the What's new article on the |
Thank! I'll close this. |
Some symbols where only added in 3.5.2 and this information was missing in the docs