PEP 655: Add Discussions-To link to the actual thread

[CAM-Gerlach]

A followup to the issue just raised on Discourse and #2266 . Currently, PEP 655 doesn't actually link the canonical discussion thread, requiring community members interesting in reading or contributing to the discussion to Google and guess to find the official discussion, or fragmenting it into multiple different threads, making following and reviewing it more difficult for both SC members/PEP-delegates and the community alike. This PR links what I believe is the correct thread, resolving this issue.

As an enhancement to the previous approach, and tested and confirmed with both build systems and our linting suite, it uses an inline link, which keeps the conciseness and human-readability of just stating the mailing list name, while still including the actual link.

CC: @davidfstr

[AA-Turner]

it uses an inline link, which keeps the conciseness and human-readability of just stating the mailing list name

Is this a real concern? I'm wary about adding a third way of writing discussions-to -- we currently have direct thread link, and mailing list (highly discouraged).

Thinking about a silly example perhaps, but if I want to write a simple script to get all discussions-to links from PEPs, currently as it is alwasys just raw text it is very simple. Adding reST markup makes it far less straightforward, although of course still possible.

A

[CAM-Gerlach]

Is this a real concern? I'm wary about adding a third way of writing discussions-to -- we currently have direct thread link, and mailing list (highly discouraged).

Its certainly not a high priority, its just that the mailing list links (unlike Discourse) are very long and noisy, requiring click-through or complex visual parsing for the user to figure out what mailing list the discussion is on, look rather ugly and take up two lines at the very top of the PEP instead of one. I don't think we could prioritize slighly simplifying a hypothetical backed script use case over making the user-facing output cleaner and more informative.

We are not (yet) programmatically parsing these, and this wouldn't make it meaningfully more complex if we did, since if < is present in the discussions_to string, then the link or email address can be found via re.search("<(.+)>", discussions_to); otherwise its just discussions_to. Likewise, if we did need to disambiguate the three types for some reason, a simple but very effective hueristic is if ` is present, its a reST link, otherwise if http: is present its a plain link, otherwise its an email/unstructured. This also stores the venue name, like the old format, which would be extractable via re.search("[`]?(.+) ?<", discussions_to) in either one.

[davidfstr]

It's not clear to me why these extra formatting changes are here. Perhaps your editor is autoformatting whitespace?

[davidfstr]

(Ditto for additional formatting changes below in the diff.)

[CAM-Gerlach]

Yeah, my editor automatically cleans up stray trailing whitespace on save, which it looks like this PEP had. I didn't see anything else in flight touching on this PEP and expected a fairly quick merge, so when I noticed it I didn't immediately go back and rewrite the commit without the whitespace changes. But if you are currently working on additional changes that this might cause a merge conflict with, I can go ahead and do that—just let me know.

[AA-Turner]

Its certainly not a high priority, its just that the mailing list links (unlike Discourse) are very long and noisy, requiring click-through or complex visual parsing for the user to figure out what mailing list the discussion is on, look rather ugly and take up two lines at the very top of the PEP instead of one.

#2351 solves this for all such links, enabling keeping the "plain" urls and also having nice rendered display

A

[CAM-Gerlach]

Good thinking; that's a better indeed a better approach than hardcoding a reST link here.

I'll make this a plain link again, but there are a few issues with the implementation on #2351 , which I submitted in a (belated) review—I'm happy to help contribute and test a fix for your review, if you prefer.

[CAM-Gerlach]

FYI @AA-Turner , there was an unresolved conversation above with the PR author, @davidfstr ... I hope I didn't result in any unnecessary merge conflicts if he's done more work on the PR in the meantime.

[davidfstr]

I didn't result in any unnecessary merge conflicts if he's done more work on the PR in the meantime.

Nah, it's fine. Normally I just prefer that all changes in a commit (including things like "Reformatted some trailing whitespace") be mentioned in the associated PR/commit comment if they are intentional.