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

#24458: Documentation for PEP 489

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 6 months ago by encukou
Modified:
4 years, 5 months ago
Reviewers:
ncoghlan
CC:
brett.cannon, Nick Coghlan, encukou, docs_python.org, devnull_psf.upfronthosting.co.za, eric.snow
Visibility:
Public.

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

Messages

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.

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst
File Doc/c-api/module.rst (right):

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode134
Doc/c-api/module.rst:134: or request "multi-phase initialization" by returning
definition struct itself.
Typo, should be: "... returning the definition struct ..."

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode245
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__``.

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode263
Doc/c-api/module.rst:263: ``m_slots``. Before it is returned, the PyModuleDef
instance must be
Monospace for ``PyModuleDef``

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode276
Doc/c-api/module.rst:276: PyModuleDef_Slot structures:
Monospace for ``PyModuleDef_Slot``

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode301
Doc/c-api/module.rst:301: The function receives a ModuleSpec instance, as
defined in PEP 451,
Cross reference to
https://docs.python.org/3/library/importlib.html?#importlib.machinery.ModuleSpec
from ModuleSpec

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode302
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.

http://bugs.python.org/review/24458/diff/15091/Doc/c-api/module.rst#newcode315
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+