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

Set up GitHub Autolink feature for this repo #2255

Closed
CAM-Gerlach opened this issue Jan 20, 2022 · 31 comments
Closed

Set up GitHub Autolink feature for this repo #2255

CAM-Gerlach opened this issue Jan 20, 2022 · 31 comments
Assignees

Comments

@CAM-Gerlach
Copy link
Member

I noticed the GitHub Autolink references feature and thought it might be a great fit for the PEPs repo—references to PEPs in GitHub issues, PRs, etc could automatically turn into links to the PEPs; same for BPOs, RFCs, and any others we want.

Any objections? Any other ideas beyond PEPs, BPOs and RFCs (@hugovk ?)

@AA-Turner
Copy link
Member

BCPs? (e.g. BCP 47)

@AA-Turner
Copy link
Member

I think this is also something we should just enable and add to later if anyone has any new ideas (this would be a great place to suggest them)

@hugovk
Copy link
Member

hugovk commented Jan 20, 2022

Any objections? Any other ideas beyond PEPs, BPOs and RFCs (@hugovk ?)

Nice idea, these are a good start.

https://github.com/python/cpython already does at least BPO- and GH-. Can we check what that full list is?

(These need to be defined at the repo level, give this ticket a +1 for org-level suport.)

@AA-Turner
Copy link
Member

https://github.com/python/cpython already does at least BPO- and GH-. Can we check what that full list is?

If you can't as a triager, it might only be repo admins -- pinging @Mariatta @brettcannon

@Mariatta
Copy link
Member

https://github.com/python/cpython already does at least BPO- and GH-. Can we check what that full list is?

On the CPython repo, we only set up the bpo-123 → https://bugs.python.org/issue123 autolink.
GH- is just built-in functionality.

@CAM-Gerlach CAM-Gerlach removed their assignment Jan 20, 2022
@CAM-Gerlach
Copy link
Member Author

If you can't as a triager, it might only be repo admins -- pinging @Mariatta @brettcannon

It has to be a repo admin. Since the PEP editors' Admin permissions were just dropped to Maintain, I am not able to do this anymore; it will have to be @gvanrossum , @warsaw or @brettcannon .

@brettcannon
Copy link
Member

What do people want specifically? Mapping for bpo- and pep-?

@CAM-Gerlach
Copy link
Member Author

bpo-NNN and PEP NNN at minimum; RFC (and maybe BCP, though I'm not sure I've seen one referenced) as a plus

@AA-Turner
Copy link
Member

AA-Turner commented Jan 22, 2022

"RFC" seems to have been used in 16 issues or PRs 1 (~0.7% of the total), and sometimes as a synonym to PEP. BCP has been used in two issues other than this one, both times by me. So really PEP and bpo- seem to be of value here, others far less so.

A

Footnotes

  1. https://github.com/python/peps/issues?q=RFC

@gpshead
Copy link
Member

gpshead commented Jan 23, 2022

If we start autolinking RFC to the IETF RFCs, people will start using it for the correct thing more often. I'm one who has mentioned RFCs in issues and CLs in the past; it's annoying to have to paste the link to them in instead. I'd keep RFC on the list.

@merwok
Copy link
Member

merwok commented Jan 23, 2022

I know RFC, BCP and ID (internet draft), but what is CL?

@JelleZijlstra
Copy link
Member

I believe it's changelist, Googlespeak for PR.

@CAM-Gerlach
Copy link
Member Author

I've never heard of CL either until now; does it link somewhere specific and public (some Google monorepo)?

@gpshead
Copy link
Member

gpshead commented Jan 24, 2022

i couldn't for the life of me figure out why you were talking about CLs here until I reread my above comment. ;) yes it just means changelist or commit or PR. I believe it originates from Perforce. Ignore that I used that acronym.

@brettcannon
Copy link
Member

You got PEP-, bpo-, and RFC-.

@CAM-Gerlach
Copy link
Member Author

This is a test

PEP 8 bpo-12345 RFC 3339

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Jan 25, 2022

This is another test

PEP-8 bpo 12345 RFC-3339

PEP-0008

@brettcannon
Copy link
Member

Did I pass the test? 😉

@Rosuav
Copy link
Contributor

Rosuav commented Jan 25, 2022

The ones with spaces didn't autolink, but other than that, they are links both here in the web UI, and in the email that GitHub sent me. 👍

@AA-Turner
Copy link
Member

PEP 8

RFC 3339

I assume this is a limitation of the autolink feature :(

Thanks Brett!

@CAM-Gerlach
Copy link
Member Author

@brettcannon Thanks, but not quite—PEP and RFC contain a spurious dash instead of a space (it should be PEP , not PEP-, unless GitHub does not allow you to inject a space) and the PEP links don't work without manually zero-padding them in the text (so I'm not sure they're worth including, since users won't know to do that), if they are not a four digit PEP. Maybe we could have a redirect rule from the non-zero-padded to the zero padded version (which would also make things a little easier when entering links manually)? @AA-Turner , any ideas?

@JelleZijlstra
Copy link
Member

Unfortunately the PEP-8 link 404s.

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Jan 25, 2022

Unfortunately the PEP-8 link 404s.

Yeah, that's because of the lack of zero padding—the PEP-0008 link works fine. If we had a redirect rule from non-zero-padded to zero-padded versions of the non-4-digit PEP URLs, that would fix the issue (since there's no way I know of to do so through GitHub). Otherwise, might not be worth doing since it won't work in most cases unless users know and remember the trick.

@AA-Turner
Copy link
Member

a redirect rule from the non-zero-padded to the zero padded version

I'd be wary of adding this. The zero padded form has been around basically forever and people are used to it -- and in the future it would be another set of redirect rules (when peps-but-even-better.python.org gets made!)

A

@brettcannon
Copy link
Member

PEP and RFC contain a spurious dash instead of a space (it should be PEP , not PEP-

It's not spurious but required. The autolinking feature will not work without the dash.

PEP links don't work without manually zero-padding them in the text

Once again, nothing to be done here on the GitHub side.

I'm not sure they're worth including, since users won't know to do that

Since you have to include the dash there's nothing to worry about. Either you get the linking or you don't, and you simply need to know about the zero padding.

The zero padded form has been around basically forever and people are used to it -- and in the future it would be another set of redirect rules (when peps-but-even-better.python.org gets made!)

I agree. People will just to live with the imperfection of this or there won't be any autolinking. This is way too much of an edge case to set up redirects for.

@CAM-Gerlach
Copy link
Member Author

It's not spurious but required. The autolinking feature will not work without the dash.

Got it, thanks.

Since you have to include the dash there's nothing to worry about. Either you get the linking or you don't, and you simply need to know about the zero padding.

I guess that makes sense, avoiding frustration while allowing those "in the know" to use them. My only concern though is that it might confuse some people into thinking PEP-NNNN is the canonical form instead of PEP nnn, but the impact will probably be pretty minor.

I agree. People will just to live with the imperfection of this or there won't be any autolinking. This is way too much of an edge case to set up redirects for.

Yeah; I run into it a lot when manually typing/modifying PEP links, but that's very much an edge case too.

@CAM-Gerlach
Copy link
Member Author

@brettcannon could you update the autolink config to use the new canonical URLs?

@JelleZijlstra
Copy link
Member

I have permissions for that now, let me do it.

@JelleZijlstra
Copy link
Member

Done PEP-0526

@JelleZijlstra
Copy link
Member

PEP-526 404s though, did we not end up creating redirects for that?

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Mar 23, 2022

Thanks @JelleZijlstra !

PEP-526 404s though, did we not end up creating redirects for that?

Not yet; see #2420 . Since it appears the - is required for GitHub autolink feature to work, it isn't as a big a deal as it would be since its already not the canonical format for referring to PEPs (and thus I typically separate the reference from the prose mention, e.g. PEP 526 (PEP-0526) to make that more clear and avoid any confusion.

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

No branches or pull requests

9 participants