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

bpo-33649: Add high-level APIs cheat-sheet by 1st1 · Pull Request #9319 · python/cpython · GitHub

/ cpython Public

bpo-33649: Add high-level APIs cheat-sheet - #9319

Merged
1st1 merged 2 commits into
python:masterfrom
1st1:docsimp5
Sep 14, 2018
Merged

bpo-33649: Add high-level APIs cheat-sheet#9319
1st1 merged 2 commits into
python:masterfrom
1st1:docsimp5

Conversation

1st1 commented Sep 14, 2018
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

bedevere-bot added docs Documentation in the Doc dir awaiting merge labels Sep 14, 2018
1st1 requested review from willingc and removed request for gpshead and rhettinger September 14, 2018 20:55
1st1 added the skip news label Sep 14, 2018

ezio-melotti left a comment
edited
Loading

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

There are a few occurrences of asyncio and some classes that have no specific markup. Usually they should either be linkified, or use ``...`` without creating a link.

Comment thread Doc/library/asyncio-api-index.rst Outdated
Comment thread Doc/library/asyncio-api-index.rst Outdated
Comment thread Doc/library/asyncio-api-index.rst Outdated

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 haven't seen the rendered version but, based on the content, I think something like 30/70 might look better.
Also using the simple table markup (http://docutils.sourceforge.net/docs/user/rst/quickref.html#tables) might make the source shorter and more readable (same below).

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

Looks pretty good rendered.

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

Adjusting to @ezio-melotti's suggested width would shorten the first entry.

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

This is pretty much what I expected: it has too much whitespace on the first column and leaves less space for the description (e.g. the first description has to take two lines instead of one).
I think the simple table markup also figures out the column width automatically based on the content (and the list-table probably does the same if you omit the width, unless you want consistency between the tables).

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

Screenshot at 30/70

Copy link
Copy Markdown
Member 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

I think the simple table markup also figures out the column width automatically based on the content (and the list-table probably does the same if you omit the width, unless you want consistency between the tables).

No, it doesn't. The table is formatted this way because I configured the columns to be a 50/50 split, which I experimentally found to look the least awful. Unfortunately, while sphinx allows to set percentages for the table columns' widths it doesn't allow to do so for the table itself. Anyways, I'd like to keep the makup as is in this PR.

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

Fair enough.
Just out of curiosity, what did you find awful, and what were you trying to achieve (consistent column width between tables, full width table, others)?
(FWIW, of the two screenshots posted by Carol (thanks!), I prefer the 30/70.)

Comment thread Doc/library/asyncio-api-index.rst Outdated

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

It is a separate exception that doesn't inherit from it?

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

Yes, sadly

Comment thread Doc/library/asyncio-api-index.rst Outdated

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

willingc 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

Fantastic 🐍

asvetlov 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

Well done!

Comment thread Doc/library/asyncio.rst Outdated

1st1 commented Sep 14, 2018

Copy link
Copy Markdown
Member Author

There are a few occurrences of asyncio and some classes that have no specific markup. Usually they should either be linkified, or use ... without creating a link.

There are plenty of links to these classes 2-3 lines around those occurrences. Let's not add too many links.

1st1 merged commit 7372c3b into python:master Sep 14, 2018
1st1 deleted the docsimp5 branch September 14, 2018 22:11
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL