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

Infra: Have PEP 0 index by the name in the PEP #3295

Merged
merged 7 commits into from
Sep 1, 2023

Conversation

thejcannon
Copy link
Contributor

@thejcannon thejcannon commented Aug 17, 2023

As discussed in https://discuss.python.org/t/meta-should-the-peps-page-index-by-not-just-the-last-name/27918/ this change makes it so the generated PEP 0000 now just uses the name as written in the PEP (After all, that's the name that's given 😄 ).

As it turns out, this is a great simplification of the code as well, and therefore this change also encompasses a whole lotta deletions.


📚 Documentation preview 📚: https://pep-previews--3295.org.readthedocs.build/

@CAM-Gerlach CAM-Gerlach changed the title Infra: Have PEP 0000 index by the name in the PEP Infra: Have PEP 0 index by the name in the PEP Aug 17, 2023
@CAM-Gerlach CAM-Gerlach added the infra Core infrastructure for building and rendering PEPs label Aug 17, 2023
@AA-Turner
Copy link
Member

It seems I missed that discourse thread -- perhaps we could add some JavaScript to make sorting configurable, rather than removing the last-name sort, which is the standard ordering for names.

A

@hugovk
Copy link
Member

hugovk commented Aug 17, 2023

(Updated from main, which should fix the Sphinx build.)

@hugovk
Copy link
Member

hugovk commented Aug 17, 2023

perhaps we could add some JavaScript to make sorting configurable, rather than removing the last-name sort, which is the standard ordering for names.

Ah, but the standard ordering where?

My surname would be sorted under K in the Netherlands, but (inconsistently) under V in most other places. Where should we put it? We can add custom code so it's sorted to my preference, but we can't predict where the reader will look for it.

And what is a last name? Hungarian and East Asian names can have family names first. Some people don't have or use last names.

Sorting by the string the person gave us is simpler all around.

See also https://www.kalzumeus.com/2010/06/17/falsehoods-programmers-believe-about-names/ for plenty more gotchas.

@Rosuav
Copy link
Contributor

Rosuav commented Aug 17, 2023

TBH I don't think I have ever cared about the exact sort order of names in PEP 0. It's something to search, not sort, and any logical ordering will make sense. Definitely in favour of using entire names.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AA-Turner AA-Turner self-requested a review August 26, 2023 09:00
@thejcannon
Copy link
Contributor Author

@AA-Turner you're busy! 😂

Should we add some kind of lint to the repo to ensure the authors follow some expected pattern?

@thejcannon
Copy link
Contributor Author

Oh I see, it's variations of some name. Hmm not so easy to catch. Nevermind 😄

@AA-Turner
Copy link
Member

AA-Turner commented Sep 1, 2023

The problem isn't the pattern, but duplicates -- the AUTHOR_OVERRIDES file that you've deleted in this PR ensured that authors were grouped together correctly -- this is now a non-issue as the source author metadata is standardised.

@AA-Turner
Copy link
Member

I've a follow-up PR to this with some refactorings that I'll open after merging this one.

A

@AA-Turner AA-Turner merged commit 32a92bd into python:main Sep 1, 2023
17 checks passed
@AA-Turner
Copy link
Member

Thanks, Josh!

A

@thejcannon thejcannon deleted the jcannon/fullname branch September 1, 2023 15:12
@thejcannon
Copy link
Contributor Author

No, thanks to y'all for letting me contribute and allowing me this change.

Relatedly I sent PEP 712 off to the SC, so I'm excited to see my name on the row 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Core infrastructure for building and rendering PEPs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants