GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — PEP 12: Update guidance on linking PEPs/RFCs per recent changes, and add check for it by CAM-Gerlach · Pull Request #2291 · python/peps

Conversation

@CAM-Gerlach

@CAM-Gerlach CAM-Gerlach commented Feb 1, 2022

Copy link
Copy Markdown
Member

In #2209 , @AA-Turner removed the legacy implicit PEP/RFC link logic, and batch-converted existing PEPs to use explicit :pep: and :rfc:: roles to reference other PEPs and RFCs, rather than relying on hacky, fragile and limited implicit links, or hardcoding URLs (often in footnotes). However, PEP 12 has not been updated accordingly to reflect this, still containing a substantial quantity of outdated language on this point.

Therefore, it was updated to update these now-misleading descriptions and examples, describe the new practice and explain and give examples of its use. This also, as a necessary side effect, addresses the most straightforward and obvious aspects of #2130 , though I will address the remaining parts of it not directly concerned with PEP/RFC links and footnotes thereof in another separate PR immediately after this one is merged, to keep things focused here.

Furthermore, some users may still be tempted to do the latter, whether due to the limitations of GitHub's rendered previews (as saw in #2282 ), or simply being unaware of the role's existence. Ensuring we always reference PEPs with the appropriate roles rather than hard links is simpler, more concise and more reliable, ensures they are formatted correctly and (with the new build system proposed in PEP-0676) allows us to do things like automatically including the PEP title in the link titletext, as users have requested.

Therefore, as originally prompted by @AA-Turner 's suggestion on #2282 , this PR adds a cheap Pre-Commit pygrep check for direct hardcoded URLs to other PEPs and RFCs. This avoids regressions in future PEPs (and spot-fixes the couple remaining instances not caught by that PR) and allowing authors and reviewers to easily spot where this occurs and replace it with the simpler, more concise, more reliable and more functional roles.

As a historical note, these changes were originally included in #2286 , but @AA-Turner and @JelleZijlstra wisely suggested splitting them out, as they deserved to be addressed separately.

  • Describe using :pep: and :rfc: roles in PEP 12, and update/remove outdated references section content
  • Add new pre-commit check for bare/direct PEP and RFC links
  • Update a couple PEPs that still used the deprecated PEP/RFC references

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
CAM-Gerlach added 2 commits January 31, 2022 21:43
@CAM-Gerlach
PEP 12: Update to specify using the :pep: role to link to other PEPs
5ee118a
@CAM-Gerlach
Add checks for hardcoded direct links to PEPs and RFCs
f86e321
@CAM-Gerlach
CAM-Gerlach requested review from AA-Turner, JelleZijlstra and hugovk February 1, 2022 04:15
@CAM-Gerlach
CAM-Gerlach requested review from a team, brettcannon, gvanrossum and warsaw as code owners February 1, 2022 04:15
@CAM-Gerlach CAM-Gerlach self-assigned this Feb 1, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 1, 2022
@CAM-Gerlach CAM-Gerlach mentioned this pull request Feb 1, 2022
Upgrade lint infra & add a few more conservative built-in checks #2286
Merged
8 tasks done
JelleZijlstra
JelleZijlstra approved these changes Feb 1, 2022
View reviewed changes
hugovk
hugovk approved these changes Feb 1, 2022
View reviewed changes
AA-Turner
AA-Turner reviewed Feb 2, 2022
View reviewed changes
Comment thread pep-0427.txt
See

- https://datatracker.ietf.org/doc/html/rfc7515
- :rfc:`7515`

@AA-Turner AA-Turner Feb 2, 2022

Copy link
Copy Markdown
Member

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.

Only comment here (also, no clue if it matters or not but to give background) I kept this as a full link to go with the other two in this section and below, which are internet drafts and so don't have the nice RFC role.

Equally, this is a triviality.

A

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@CAM-Gerlach CAM-Gerlach Feb 2, 2022

Copy link
Copy Markdown
Member Author

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.

Hmm, I see. To make them more closely match, we could just tweak the other two instead, to something like

- `Internet-Draft: JSON Private Key <https://datatracker.ietf.org/doc/html/draft-jones-jose-json-private-key.html>`

To note, I-Ds are intentionally made to not be too friendly to easily reference, since you really aren't "supposed" to outside the context of an explicit citation as a work in progress (and the two mentioned are now expired).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
AA-Turner
AA-Turner approved these changes Feb 2, 2022
View reviewed changes

@AA-Turner AA-Turner left a comment

Copy link
Copy Markdown
Member

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.

Barring the comment above (clicked the wrong button), which we might not even want to do anything about, this looks good, thanks Cam.

A

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@JelleZijlstra

JelleZijlstra commented Feb 3, 2022

Copy link
Copy Markdown
Member

@AA-Turner and @CAM-Gerlach you might be interested in this project (by some of the people working on the CPython docs): https://github.com/sphinx-contrib/sphinx-lint

CAM-Gerlach reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@CAM-Gerlach

CAM-Gerlach commented Feb 3, 2022

Copy link
Copy Markdown
Member Author

@JelleZijlstra Thanks! On Spyder-Docs we currently use rstcheck as an additional linter (besides a comprehensive suite for the HTML, CSS, JS, Jinja and various file formats, and other misc checks), but its not super well maintained nowadays (e.g. Python 3.10 support) and apparently mostly relies on docutils warnings (which we can check ourselves in the build system, once we ensure the current docs build cleanly), and checking the syntax of embedded code blocks, which it looks like sphinx-lint also does along with other checks.

Once this PR is merged, I can go ahead and look into that as well as potentially a few more custom checks which PEP authors have requested, such as using non-URLs in implicit references without a trailing underscore, and even multiple PEP editors have gotten confused about.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
JelleZijlstra merged commit 52ffaeb into python:main Feb 3, 2022
@CAM-Gerlach CAM-Gerlach mentioned this pull request Feb 4, 2022
PEP 12: Recommend and explain native RST hyperlinks (versus footnotes) #2302
Merged
@erlend-aasland erlend-aasland mentioned this pull request May 13, 2022
pep8/greppable exception messages erlend-aasland/peps#1
Closed
@erlend-aasland erlend-aasland mentioned this pull request Jun 27, 2022
pep 687/mark as accepted erlend-aasland/peps#2
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@hugovk hugovk hugovk approved these changes
@AA-Turner AA-Turner AA-Turner approved these changes
@brettcannon brettcannon Awaiting requested review from brettcannon
@gvanrossum gvanrossum Awaiting requested review from gvanrossum
@warsaw warsaw Awaiting requested review from warsaw

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@CAM-Gerlach CAM-Gerlach

Labels

CLA signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@CAM-Gerlach @JelleZijlstra @hugovk @AA-Turner @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.