Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in

#24458: Documentation for PEP 489

Can't Edit
Can't Publish+Mail
Start Review
4 years, 6 months ago by encukou
4 years, 5 months ago
brett.cannon, Nick Coghlan, encukou, docs_python.org, devnull_psf.upfronthosting.co.za, eric.snow

Patch Set 1 #

Total comments: 7

Patch Set 2 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/c-api/init.rst View 1 1 chunk +2 lines, -0 lines 0 comments Download
Doc/c-api/module.rst View 1 8 chunks +243 lines, -77 lines 0 comments Download
Doc/extending/building.rst View 1 1 chunk +47 lines, -16 lines 0 comments Download
Doc/extending/extending.rst View 1 1 chunk +7 lines, -0 lines 0 comments Download


Total messages: 1
Nick Coghlan
4 years, 5 months ago #1
Sorry for the delay reviewing this - the beta release revealed some aspects of
the async/await changes we thought could be deferred to 3.6 really needed to be
addressed for 3.5 after all.

Back on topic, I really like the revised structure for these docs.

Mostly some minor comments regarding typos and formatting, one substantive
question, but there's one substantive suggestion about providing some rationale
for why we read the module name from the spec by default, and not from the C
level definition.

File Doc/c-api/module.rst (right):

Doc/c-api/module.rst:134: or request "multi-phase initialization" by returning
definition struct itself.
Typo, should be: "... returning the definition struct ..."

Doc/c-api/module.rst:245: The distinction is similar to the __new__ and __init__
methods of classes.
These don't need to be cross-references, but should probably be flagged as
monospace: ``__new__`` and ``__init__``.

Doc/c-api/module.rst:263: ``m_slots``. Before it is returned, the PyModuleDef
instance must be
Monospace for ``PyModuleDef``

Doc/c-api/module.rst:276: PyModuleDef_Slot structures:
Monospace for ``PyModuleDef_Slot``

Doc/c-api/module.rst:301: The function receives a ModuleSpec instance, as
defined in PEP 451,
Cross reference to
from ModuleSpec

Doc/c-api/module.rst:302: and the PyModuleDef instance.
Monospace for ``PyModuleDef``. I won't comment on any more of these, you get the
idea :)

However, I also belatedly note that
https://docs.python.org/devguide/documenting.html is silent on the question of
whether or not to monospace names when not cross-referencing them. Up to you if
you'd prefer to seek clarification from the docs mailing list before deciding
one way or the other.

Doc/c-api/module.rst:315: definition.
Perhaps worth mentioning that this allows extension modules to dynamically
adjust to their place in the module hierarchy and be imported under different
names through symlinks, all while sharing a single module definition.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+