-
-
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
Infra: Use Docutils' list-table
syntax for PEP 0
#2616
Conversation
The following commit authors need to sign the Contributor License Agreement: |
b1fa456
to
eff422b
Compare
eff422b
to
b05b2fb
Compare
Save for the CLA check, this is now done -- identical output in PEP 0, and a nice reduction in complexity as (almost) all of the custom PEP 0 transform is now gone (only the email address masking logic remains). A |
*Not quite identical output to PEP 0, PEP titles are no longer truncated, and PEP 604's title now links correctly. A |
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.
It would be nice to have some tests for the changed emit_*
functions in writer.py
.
(Testing just emit_pep_category
would also cover emit_column_headers
and emit_pep_row
.)
self.emit_text(" - PEP") | ||
self.emit_text(" - PEP Title") | ||
self.emit_text(" - PEP Author(s)") |
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.
"Author(s)" is a bit ugly, for a table heading can we just put "Authors" or "Author"?
Also I think it's clear that the title is the PEP title, and the authors are the PEP authors, in a list of PEPs on the PEP index page on https://peps.python.org/ :)
self.emit_text(" - PEP") | |
self.emit_text(" - PEP Title") | |
self.emit_text(" - PEP Author(s)") | |
self.emit_text(" - PEP") | |
self.emit_text(" - Title") | |
self.emit_text(" - Authors") |
list-table
syntax for PEP 0list-table
syntax for PEP 0
I'm not sure what these would look like -- there's no real point to unit tests of just the function here, as what matters is that the entire list is a valid input to the I'd then argue that the integration test part is handled by if it is parsed correctly as reST! A |
(Also, the CLA is still not working -- just tried again) |
Yeah, tests not so valuable here. If nothing else, a test would show the code is at least running without exceptions, although we should probably notice it (but if not a super simple test would have helped ;) CLA: I've pinged @ambv again. |
Change in support of subindices.
A