Skip to content
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 12: Update guidance on linking PEPs/RFCs per recent changes, and add check for it #2291

Merged
merged 2 commits into from
Feb 3, 2022

Conversation

CAM-Gerlach
Copy link
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

@@ -312,9 +312,9 @@ checker only needs to establish that RECORD matches the signature.

See

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

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

Copy link
Member Author

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).

Copy link
Member

@AA-Turner AA-Turner left a 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

@JelleZijlstra
Copy link
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
Copy link
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants