FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-40896: add missing links to source-code to library documentation by edison12a · Pull Request #20843 · python/cpython · GitHub

/ cpython Public

bpo-40896: add missing links to source-code to library documentation - #20843

Closed
edison12a wants to merge 2 commits into
python:mainfrom
edison12a:bpo-40896
ConversationCommits (2)ChecksFiles changed
Closed

bpo-40896: add missing links to source-code to library documentation#20843
edison12a wants to merge 2 commits into
python:mainfrom
edison12a:bpo-40896

Conversation

edison12a commented Jun 12, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

edison12a requested review from 1st1 and asvetlov as code owners June 12, 2020 21:20
bedevere-bot added docs Documentation in the Doc dir awaiting review labels Jun 12, 2020
remilapeyre suggested changes Jun 12, 2020
View reviewed changes

remilapeyre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @SimiCode, except curses.panel.rst that contains no code I think you got them all.

Comment thread Misc/NEWS.d/next/Documentation/2020-06-12-21-23-23.bpo-40896.mIMSEP.rst
@@ -0,0 +1 @@
Add links to Library source code to Documentation. Enhanced by Edison Abahurire. No newline at end of file

remilapeyre Jun 12, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Documentation only changes don't need a blurb.

terryjreedy Jun 13, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

But they can and for non-trivial changes ofter do. 'Documentation' is a section in the blurb template and in the changelog.
https://docs.python.org/3/whatsnew/changelog.html#changelog
I would change and expand to
"Add source links to doc for 2to3, asyncio/, ..., curses/, ... wsgiref."

aeros Jun 13, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think in this case it could be reasonable either way, but I wouldn't consider it to be strictly required for small documentation enhancements.

Comment thread Doc/library/curses.panel.rst

.. sectionauthor:: A.M. Kuchling <amk@amk.ca>

**Source code:** :source:`Lib/curses/panel.py`

remilapeyre Jun 12, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This one is not really useful.

edison12a Jun 13, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@remilapeyre Why, Has it been deprecated?

remilapeyre Jul 20, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not, it's just that the file is a wrapper for the C module: https://github.com/python/cpython/blob/master/Lib/curses/panel.py

aeros suggested changes Jun 13, 2020
View reviewed changes

aeros left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the PR @SimiCode.

In the case of the asyncio documentation changes, some of the source code links were intentionally omitted as not being particularly relevant or additionally useful for the reader's understanding. Also, in the case of the link to the source code for asyncio, it's already displayed in the footer of the documentation and doesn't need to be displayed at the top as well.

For more details, see Yury's review comments on my PR that was merged last year: #16640.

bedevere-bot commented Jun 13, 2020

Copy link
Copy Markdown

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

terryjreedy requested changes Jun 13, 2020
View reviewed changes

terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We started with a few, then some more, and then some more. I am sure some of these were intentionally left out, but the line is fuzzy and the added space in the doc small. Where the package has a maintainer, you might ask if there is a really good reason for the omission.

Comment thread Doc/library/formatter.rst
.. deprecated:: 3.4
Due to lack of usage, the formatter module has been deprecated.

**Source code:** :source:`Lib/formatter.py`

terryjreedy Jun 13, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I an not sure that adding this for a deprecated module is a good idea.

Comment thread Misc/NEWS.d/next/Documentation/2020-06-12-21-23-23.bpo-40896.mIMSEP.rst
@@ -0,0 +1 @@
Add links to Library source code to Documentation. Enhanced by Edison Abahurire. No newline at end of file

terryjreedy Jun 13, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

But they can and for non-trivial changes ofter do. 'Documentation' is a section in the blurb template and in the changelog.
https://docs.python.org/3/whatsnew/changelog.html#changelog
I would change and expand to
"Add source links to doc for 2to3, asyncio/, ..., curses/, ... wsgiref."

remilapeyre commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

Hi @SimiCode, thanks for your work! Please address the last comments of the code review.

iritkatriel reviewed Oct 20, 2021
View reviewed changes
Comment thread Doc/library/2to3.rst

.. sectionauthor:: Benjamin Peterson <benjamin@python.org>

**Source code:** :source:`Lib/lib2to3`

iritkatriel Oct 20, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Also deprecated.

slateny commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

@edison12a Would you still be interested in addressing some of the comments above? I can clone and create a new PR if not.

simicode mannequin mentioned this pull request Apr 10, 2022
Closed

slateny commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

If you'd like to resume the PR let me know and I can reopen. In the meanwhile, see #99363 instead.

slateny closed this Nov 11, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

iritkatriel iritkatriel left review comments
terryjreedy terryjreedy requested changes
1st1 Awaiting requested review from 1st1 1st1 is a code owner
asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
+2 more reviewers
remilapeyre remilapeyre requested changes
aeros aeros requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting changes docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL