Repository navigation
Lint: Allow per-message Post-History links w/trailing slash - #3005
Merged
CAM-Gerlach merged 5 commits intoFeb 6, 2023
Merged
Conversation
CAM-Gerlach
requested review from
ericsnowcurrently and
hugovk
as code owners
February 4, 2023 00:05
Merged
3 of 4 tasks
Member
Author
|
Also, to reduce any reader confusion about what each link points to, I modified the the link type shown in the tooltip when hovering over links to individual messages to say
|
hugovk
approved these changes
Feb 6, 2023
pradyunsg
approved these changes
Feb 6, 2023
ericsnowcurrently
approved these changes
Feb 6, 2023
ericsnowcurrently
left a comment
ericsnowcurrently
left a comment
Member
There was a problem hiding this comment.
LGTM
Thanks for doing this!
Member
Author
|
Thanks for your great feedback @ericsnowcurrently ! |
CAM-Gerlach
changed the title
This was referenced Mar 12, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
In PR #3004 , a regression was introduced in the linter, bypassing the check that Discourse links are to the thread rather than to an individual message (as the modern link form of
Post-Historyis for keeping a history of theDiscussions-Tothreads, rather than links to individual messages about the PEP, which can go in the PEP body if desired).This is also a pretty easy mistake to make by accident, as if you've scrolled past the first message, the message number will be appended to the URL, leaving readers unintentionally stuck in the middle of the thread rather than at the start (which also messes with Discourse's bookmarking of where you left off reading a thread).
Finally, as the headers are intended to be structured data in an easily-machine-parsable format, consumed by a number of tools in this repo and many more through the PEP API, including PEP-o-tron, the Python Discord bot, and others, it's best if they are as consistent as possible.