-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
PEP 676: Refine formatting, spelling and syntax, and copyedit text #2200
Changes from all commits
cd4e570
03c9c6b
6263951
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,20 +9,22 @@ Content-Type: text/x-rst | |||||||||||||||||||||||||||||||||||||||
Created: 01-Nov-2021 | ||||||||||||||||||||||||||||||||||||||||
Post-History: 23-Sep-2021 | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Abstract | ||||||||||||||||||||||||||||||||||||||||
======== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
This PEP addresses the infrastructure around rendering PEP files from | ||||||||||||||||||||||||||||||||||||||||
reStructuredText_ files to HTML webpages. We aim to specify a self-contained | ||||||||||||||||||||||||||||||||||||||||
`reStructuredText`_ files to HTML webpages. We aim to specify a self-contained | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already a |
||||||||||||||||||||||||||||||||||||||||
and maintainable solution for PEP readers, authors, and editors. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
Motivation | ||||||||||||||||||||||||||||||||||||||||
========== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
As of November 2021, Python Enhancement Proposals (PEPs) are rendered in a | ||||||||||||||||||||||||||||||||||||||||
multi-system, multi-stage process. A continuous integration (CI) task runs a | ||||||||||||||||||||||||||||||||||||||||
docutils_ script to render all PEP files individually. The CI task then uploads | ||||||||||||||||||||||||||||||||||||||||
a tar archive to a server, where it is retrieved and rendered into the | ||||||||||||||||||||||||||||||||||||||||
`docutils`_ script to render all PEP files individually. The CI task then | ||||||||||||||||||||||||||||||||||||||||
uploads a tar archive to a server, where it is retrieved and rendered into the | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above I'd drop this |
||||||||||||||||||||||||||||||||||||||||
`python.org`_ website periodically. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
This places a constraint on the `python.org`_ website to handle raw HTML | ||||||||||||||||||||||||||||||||||||||||
|
@@ -33,15 +35,15 @@ This PEP provides a specification for self-contained rendering of PEPs. This | |||||||||||||||||||||||||||||||||||||||
would: | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
* reduce the amount of distributed configuration for supporting PEPs | ||||||||||||||||||||||||||||||||||||||||
* enable quality-of-life improvements for those who read, write, and review | ||||||||||||||||||||||||||||||||||||||||
PEPs | ||||||||||||||||||||||||||||||||||||||||
* enable quality-of-life improvements for those who read, write and review PEPs | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After the change this line is 80 characters long -- one bit of PEP 12 I did follow is to keep to 79 maximum for prose! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only measure it as 79, like a number of other lines, unless you could the newline (which I thought didn't count?) But maybe I'm mistaken. |
||||||||||||||||||||||||||||||||||||||||
* solve a number of outstanding issues, and lay the path for improvements | ||||||||||||||||||||||||||||||||||||||||
* save volunteer maintainers' time | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
We propose that PEPs are accessed through peps.python.org_ at the top-level | ||||||||||||||||||||||||||||||||||||||||
namespace (for example `peps.python.org/pep-0008/`_), and that all custom | ||||||||||||||||||||||||||||||||||||||||
We propose that PEPs are accessed through `peps.python.org`_ as the top-level | ||||||||||||||||||||||||||||||||||||||||
namespace (for example, `peps.python.org/pep-0008`_), and that all custom | ||||||||||||||||||||||||||||||||||||||||
tooling to support rendering PEPs is hosted in the `python/peps`_ repository. | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, I'd drop these (the trailing forward slash might be unneeded, but its a small thing that signifies it's a directory not a file) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually dropped the trailing forward slash to—in the spirit of the PEP's strong lack of specificity elsewhere in seemingly more important areas, such as not even mentioning the name of the new build backend (Sphinx)—abstract away what I saw as an implementation detail. But I defer to your judgement on that. I can understand the stylistic choice of not putting explicit backticks around single-word references, so long as its consistently, but what originally prompted this change was the fact that you do put backticks around |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Rationale | ||||||||||||||||||||||||||||||||||||||||
========= | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
@@ -50,12 +52,12 @@ Simplifying and Centralising Infrastructure | |||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
As of November 2021, to locally render a PEP file, a PEP author or editor needs | ||||||||||||||||||||||||||||||||||||||||
to create a full local instance of the `python.org`_ website and run a number | ||||||||||||||||||||||||||||||||||||||||
of disparate scripts, following documentation_ that lives outside of the | ||||||||||||||||||||||||||||||||||||||||
of disparate scripts, following `documentation`_ that lives outside of the | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above |
||||||||||||||||||||||||||||||||||||||||
`python/peps`_ repository. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
The proposed implementation provides a single Makefile_ and a Python script to | ||||||||||||||||||||||||||||||||||||||||
render all PEP files, with options to target a web-server or local filesystem | ||||||||||||||||||||||||||||||||||||||||
environment. | ||||||||||||||||||||||||||||||||||||||||
By contrast, the proposed implementation provides a single `Makefile`_ and a | ||||||||||||||||||||||||||||||||||||||||
Python script to render all PEP files, with options to target a web server or | ||||||||||||||||||||||||||||||||||||||||
the local filesystem environment. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+58
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Using a single repository to host all tooling will clarify where to raise | ||||||||||||||||||||||||||||||||||||||||
issues, reducing volunteer time spent in triage. | ||||||||||||||||||||||||||||||||||||||||
|
@@ -64,6 +66,7 @@ Simplified and centralised tooling may also reduce the barrier to entry to | |||||||||||||||||||||||||||||||||||||||
further improvements, as the scope of the PEP rendering infrastructure is well | ||||||||||||||||||||||||||||||||||||||||
defined. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
Quality-of-Life Improvements and Resolving Issues | ||||||||||||||||||||||||||||||||||||||||
------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
@@ -74,38 +77,42 @@ There are several requests for additional features in reading PEPs, such as: | |||||||||||||||||||||||||||||||||||||||
* support for SVG images [3]_ | ||||||||||||||||||||||||||||||||||||||||
* typographic quotation marks [4]_ | ||||||||||||||||||||||||||||||||||||||||
* additional footer information [5]_ | ||||||||||||||||||||||||||||||||||||||||
* intersphinx functionality [6]_ | ||||||||||||||||||||||||||||||||||||||||
* Intersphinx functionality [6]_ | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The documentation for intersphinx has inconsistent capitalisation, and I'd argue keeping a lower case here is nicer with the rest of the list. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
These are "easy wins" from this proposal, and would serve to improve the | ||||||||||||||||||||||||||||||||||||||||
quality-of-life for consumers of PEPs (including reviewers and writers). For | ||||||||||||||||||||||||||||||||||||||||
example, the reference implementation no longer requires a scheduled render | ||||||||||||||||||||||||||||||||||||||||
example, the new reference implementation no longer requires a scheduled render | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pedantic (as with all of these!) but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may technically be redundant so long as the reader carefully reads the rest of the sentence ("no longer requires"), but I'd argue it adds a bit of clarity. However, examining it in context, it isn't so much as I thought. |
||||||||||||||||||||||||||||||||||||||||
process to run, instead initiating rendering on every commit to the | ||||||||||||||||||||||||||||||||||||||||
`python/peps`_ repository. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Equally, there are a small number of broken items, for example list styles not | ||||||||||||||||||||||||||||||||||||||||
being respected or support for updating images being challenging with the | ||||||||||||||||||||||||||||||||||||||||
system [7]_. These would be solved by default in the proposal. | ||||||||||||||||||||||||||||||||||||||||
Equally, there are a number of broken items currently; for example, list styles | ||||||||||||||||||||||||||||||||||||||||
are not respected, and support for updating images is challenging with the | ||||||||||||||||||||||||||||||||||||||||
current system [7]_. These are solved by the reference implementation of | ||||||||||||||||||||||||||||||||||||||||
this proposal. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+88
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the simplification. However:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm,
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Commercial providers such as `Read the Docs`_ can additionally enhance this | ||||||||||||||||||||||||||||||||||||||||
experience, for example by providing rendered previews of changes in pull | ||||||||||||||||||||||||||||||||||||||||
Third-party providers such as `Read the Docs`_ can additionally enhance this | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No strong feelings here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason for tweaking the diction was because the important thing about the providers weren't that they were commercial (paid services), but that they were third party (i.e. using standard Sphinx interoperates with other non-bespoke services). |
||||||||||||||||||||||||||||||||||||||||
experience; for example, by providing rendered previews of changes in pull | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semi-colons are banned! :p There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without it, it's a comma splice, which is a grammar error. Adding two commas instead results in ambiguous syntax. However, if you want to avoid the semicolon, you could restructure it a bit like this:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
requests. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Specification | ||||||||||||||||||||||||||||||||||||||||
============= | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should note that we are using MUST/SHOULD/MAY as per BCP 14 / RFC 2119
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are widely used in the specification section of other PEPs, and I've never seen it explicitly noted like this. Perhaps it would be a better idea, if this is desired, to do so in PEP 1 or PEP 12 instead, instead of redundantly including this note in every PEP. |
||||||||||||||||||||||||||||||||||||||||
The proposed specification for rendering the PEP files to HTML is as per the | ||||||||||||||||||||||||||||||||||||||||
`reference implementation`_. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
That the HTML files should be made available under peps.python.org_. The | ||||||||||||||||||||||||||||||||||||||||
rendered files may be hosted just as static files, and behind a content | ||||||||||||||||||||||||||||||||||||||||
delivery network (CDN). | ||||||||||||||||||||||||||||||||||||||||
The HTML files SHOULD be made available under the `peps.python.org`_ domain. | ||||||||||||||||||||||||||||||||||||||||
The rendered output SHOULD be hosted as static files, and MAY be behind a | ||||||||||||||||||||||||||||||||||||||||
content delivery network (CDN). | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+104
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
The following redirect rules must be created from the `python.org`_ domain: | ||||||||||||||||||||||||||||||||||||||||
The following redirect rules MUST be created for the `python.org`_ domain: | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No strong feelings, I supose I'd argue the redirect rules are being created from python.org to peps.python.org, although the purpose is for peps.python.org to work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It originally had "from" and I hadn't changed it, but @hugovk made the point that what was being described being "created" are the redirect rules, which are for the python.org domain, while the redirects themselves are from that domain to peps.python.org There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. Both are fine then, depending on how you read it :) I was reading it like instructions for the |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
* /peps/ -> https://peps.python.org/ | ||||||||||||||||||||||||||||||||||||||||
* /dev/peps/ -> https://peps.python.org/ | ||||||||||||||||||||||||||||||||||||||||
* /peps/(.*)\.html -> https://peps.python.org/$1 | ||||||||||||||||||||||||||||||||||||||||
* /dev/peps/(.*) -> https://peps.python.org/$1 | ||||||||||||||||||||||||||||||||||||||||
* ``/peps/`` -> https://peps.python.org/ | ||||||||||||||||||||||||||||||||||||||||
* ``/dev/peps/`` -> https://peps.python.org/ | ||||||||||||||||||||||||||||||||||||||||
* ``/peps/(.*)\.html`` -> https://peps.python.org/$1 | ||||||||||||||||||||||||||||||||||||||||
* ``/dev/peps/(.*)`` -> https://peps.python.org/$1 | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
The following Nginx configuration would achieve this: | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the nginx project use either NGINX or nginx, personally I'd prefer the second (capitals hurt readability). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd seen NGINX, nginx, Nginx and NginX in various official and important third-party sources, but sure let's revert to |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
.. code-block:: nginx | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
@@ -121,69 +128,84 @@ The following redirect rules must be created from the `python.org`_ domain: | |||||||||||||||||||||||||||||||||||||||
return 308 https://peps.python.org/; | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Redirects must be implemented to preserve `URL fragments`_ for backward | ||||||||||||||||||||||||||||||||||||||||
compatability purposes. | ||||||||||||||||||||||||||||||||||||||||
Redirects MUST be implemented to preserve `URL fragments`_ for backward | ||||||||||||||||||||||||||||||||||||||||
compatibility purposes. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+131
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for spotting compatability -> compatibility! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank my spellchecker... |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Backwards Compatibility | ||||||||||||||||||||||||||||||||||||||||
======================= | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Due to server-side redirects to new canonical URLs, there are no backwards | ||||||||||||||||||||||||||||||||||||||||
compatability concerns. Links in previously published materials referring to | ||||||||||||||||||||||||||||||||||||||||
any old URL scheme will be guaranteed to work. | ||||||||||||||||||||||||||||||||||||||||
Due to server-side redirects to new canonical URLs, links in previously | ||||||||||||||||||||||||||||||||||||||||
published materials referring to the old URL scheme will be guaranteed to work. | ||||||||||||||||||||||||||||||||||||||||
Furthermore, aside from the resolved bugs and new implemented features, all | ||||||||||||||||||||||||||||||||||||||||
existing PEPs will still render as they did before with the new backend. | ||||||||||||||||||||||||||||||||||||||||
Therefore, this poses no major backwards compatibility concerns. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+138
to
+142
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can argue this both ways (the outputted HTML is different, but the words are the same). I'm not sure it adds anything, I guess.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I understand my original wording wasn't the best, I have to disagree with your dismissal of its importance overall. As a reader and reviewer of the PEP, my biggest backward compat concern aside from the URL change would be whether it causes any major rendering regressions in displaying previously valid syntax/formatting used in existing PEPs. Therefore, I certainly wouldn't dismiss this entirely as a backward compat concern. While those of us involved are well aware it doesn't, making this explicit helps ensure this is clear to everyone. What about (also fixed a missing definite article):
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Security Implications | ||||||||||||||||||||||||||||||||||||||||
===================== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
No security implications, and the main `python.org`_ website will no longer | ||||||||||||||||||||||||||||||||||||||||
take raw HTML uploads, closing a potential threat vector. | ||||||||||||||||||||||||||||||||||||||||
The main `python.org`_ website will no longer need to process raw HTML | ||||||||||||||||||||||||||||||||||||||||
uploads, closing a potential threat vector, and the PEP building and | ||||||||||||||||||||||||||||||||||||||||
deployment process will use modern, well maintained code and secure | ||||||||||||||||||||||||||||||||||||||||
automated platforms, reducing the likely potential attack surface. Therefore, | ||||||||||||||||||||||||||||||||||||||||
there is no forseen negative security impact. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+148
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Took what you have, with one tweak to my writing (add a missing hyphen) and two tweaks to yours: add a missing comma (required) and sequence modern before well-maintained for better flow and balance (optional, comes down to judgement); and add "Therefore" to more explicitly connect the logic and improve the choppy flow due to three consecutive disconnected sentences. |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
How to Teach This | ||||||||||||||||||||||||||||||||||||||||
================= | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
The new canonical URLs will be publicised in the documentation. However, this | ||||||||||||||||||||||||||||||||||||||||
is mainly a backend infrastructure change, and there should be minimal | ||||||||||||||||||||||||||||||||||||||||
end-user impact. | ||||||||||||||||||||||||||||||||||||||||
end-user impact. PEP 1 and PEP 12 will be updated as needed to include and | ||||||||||||||||||||||||||||||||||||||||
document any relevant new roles, directive, syntax and other constructs enabled | ||||||||||||||||||||||||||||||||||||||||
by this change. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+160
to
+162
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see anything in PEP 1 that would change. PEP 12 I'd (personally) suggest if updated should redirect to a canonical reStructuredText tutorial and add PEP-specific exceptions or ammendments, but I don't think that in and of itself would be something requiring a PEP. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is why I put "as needed", but what about making this more generic instead and cut out the rest, i.e.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Reference Implementation | ||||||||||||||||||||||||||||||||||||||||
======================== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
The proposed implementation has been merged into the `python/peps`_ repository | ||||||||||||||||||||||||||||||||||||||||
in a series of pull requests [8]_. This automatically renders all PEPs on every | ||||||||||||||||||||||||||||||||||||||||
commit. | ||||||||||||||||||||||||||||||||||||||||
in a series of pull requests [8]_. It currently automatically renders all | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see the issue with
Suggested change
Or what about this, which would be cleaner but I'd need to apply it manually since its over the line length limit:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
PEPs on every commit. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Rejected Ideas | ||||||||||||||||||||||||||||||||||||||||
============== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
It would likely be possible to amend the current (as of November 2021) | ||||||||||||||||||||||||||||||||||||||||
rendering process to include a lot of the quality-of-life improvements and | ||||||||||||||||||||||||||||||||||||||||
issue mitigations mentioned above. However, we do not believe that this would | ||||||||||||||||||||||||||||||||||||||||
It would likely be possible to amend the current docutils-based manual | ||||||||||||||||||||||||||||||||||||||||
rendering process to include many of the quality-of-life improvements and | ||||||||||||||||||||||||||||||||||||||||
bug mitigations mentioned above. However, we do not believe that this would | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+176
to
+178
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could keep 'docutils-based' but drop the rest here -- the current process isn't manual, and I prefer the earlier wording for the other two. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually on second thoughts I'm not keen on 'docutils-based', as Sphinx itself extends docutils. Perhaps a silly reason though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can certainly understand the "manual" argument. However, "a lot" is not appropriate diction in formal nor technical writing such as this, and while it is technically correct that Sphinx extends docutils, this line does not contradict that, the existing version of the PEP already refers to the process being a "docutils script", and it is fairly clear what is meant. |
||||||||||||||||||||||||||||||||||||||||
solve the distributed tooling issue. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
It would be possible to use the output from the proposed rendering system and | ||||||||||||||||||||||||||||||||||||||||
import it into `python.org`_. We would argue however that this would be the | ||||||||||||||||||||||||||||||||||||||||
worst of both worlds, as a great deal of complexity is added, and none is | ||||||||||||||||||||||||||||||||||||||||
removed. | ||||||||||||||||||||||||||||||||||||||||
import it into the existing `python.org`_ domain. We would argue, however, | ||||||||||||||||||||||||||||||||||||||||
that this would be the worst of both worlds, as a great deal of complexity | ||||||||||||||||||||||||||||||||||||||||
is added, while none is removed. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+182
to
+184
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the earlier phrasing scans better. (Whilst it is common style to have a comma after 'however', I don't think it is strictly required. Perhaps we could do:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commas are syntactically needed there, not a just stylistic preference. However, your proposed edit acceptably side-steps the problem without requiring them. |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Open Issues | ||||||||||||||||||||||||||||||||||||||||
=========== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
None. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Acknowledgements | ||||||||||||||||||||||||||||||||||||||||
================ | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Thanks to Hugo van Kemenade, Pablo Galindo Salgado, and Éric Araujo for support | ||||||||||||||||||||||||||||||||||||||||
since April 2020. | ||||||||||||||||||||||||||||||||||||||||
Acknowledgments | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledgments (with the second 'e') is a valid spelling! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, my spellchecker flagged it (I may have worked as a copyeditor, but I've always relied mostly on automated tools for spelling) and I didn't realize UK English had a different spelling here (which I'd respected elsewhere) |
||||||||||||||||||||||||||||||||||||||||
=============== | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Thanks to Hugo van Kemenade, Pablo Galindo Salgado and Éric Araujo for | ||||||||||||||||||||||||||||||||||||||||
supporting this effort since April 2020. | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+196
to
+197
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer 'for support' -- please could you drop this one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In its previous usage, "for support" is an unambiguous grammar error. At the very minimum it needs a definite article, like this:
Suggested change
and I tweaked it further to avoid the awkward and mildly ambiguous phrasing, but its not wrong without that, per say. If you'd prefer a more conservative but still correct change, the above would suffice. |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
Footnotes | ||||||||||||||||||||||||||||||||||||||||
========= | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
.. _documentation: https://pythondotorg.readthedocs.io/pep_generation.html | ||||||||||||||||||||||||||||||||||||||||
.. _docutils: https://docutils.sourceforge.io | ||||||||||||||||||||||||||||||||||||||||
.. _Makefile: https://www.gnu.org/software/make/manual/make.html#Introduction | ||||||||||||||||||||||||||||||||||||||||
.. _peps.python.org: https://peps.python.org/ | ||||||||||||||||||||||||||||||||||||||||
.. _peps.python.org/pep-0008/: https://peps.python.org/pep-0008/ | ||||||||||||||||||||||||||||||||||||||||
.. _python.org: https://www.python.org | ||||||||||||||||||||||||||||||||||||||||
.. _peps.python.org: https://peps.python.org/ | ||||||||||||||||||||||||||||||||||||||||
.. _peps.python.org/pep-0008: https://peps.python.org/pep-0008/ | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
206
to
+208
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was sorting logically (domain -> subdomain -> url) rather than lexigraphically here, but its fine if you want to keep that, of course—I auto-lexigraphically-sorted the references section in my PEP 639, albeit that one was over 100 entries long. |
||||||||||||||||||||||||||||||||||||||||
.. _python/peps: https://github.com/python/peps | ||||||||||||||||||||||||||||||||||||||||
.. _Read the Docs: https://readthedocs.org | ||||||||||||||||||||||||||||||||||||||||
.. _reStructuredText: https://docutils.sourceforge.io/rst.html | ||||||||||||||||||||||||||||||||||||||||
|
@@ -218,12 +240,14 @@ Footnotes | |||||||||||||||||||||||||||||||||||||||
`peps#1933 <https://github.com/python/peps/pull/1933>`__, | ||||||||||||||||||||||||||||||||||||||||
`peps#1934 <https://github.com/python/peps/pull/1934>`__ | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
Copyright | ||||||||||||||||||||||||||||||||||||||||
========= | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
This document is placed in the public domain or under the | ||||||||||||||||||||||||||||||||||||||||
CC0-1.0-Universal license, whichever is more permissive. | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above (xref extra lines) |
||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
.. | ||||||||||||||||||||||||||||||||||||||||
Local Variables: | ||||||||||||||||||||||||||||||||||||||||
|
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.
Whilst PEP 12 does technically say that two blank lines are required, it also calls for e.g. double spacing, which a substantial number of PEPs ignore. I would argue it's a stylistic choice when editing documents, and would just add to git blame noise so I'd drop this (& all other similar line insertions).
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.
I'm not sure I've seen any recent PEP use 2 spaces after periods, and that convention is very uncommon in in general these days (much more so than when PEP 9/PEP 12 was written) so maybe PEP 12 should be updated on that point—I can open an issue after I finish my PR updating the references/footnotes wording. But as for two blank lines before headings, I find that as more of a readability issue, not just a style one, as I find it much easier to clearly distinguish section breaks, and see that convention or a similar one much more frequently in Markdown and reST elsewhere.
That said, I don't feel strongly about it and don't want to override your preference as the PEP author, though I'd rather hear from @JelleZijlstra or another PEP editor before going back through and redoing it.