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 727: Documentation Metadata in Typing #3310

Merged
merged 7 commits into from Aug 28, 2023

Conversation

tiangolo
Copy link
Sponsor Contributor

@tiangolo tiangolo commented Aug 28, 2023

Add PEP 727: Documentation Metadata in Typing


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

@tiangolo tiangolo marked this pull request as ready for review August 28, 2023 19:22
@tiangolo tiangolo requested a review from a team as a code owner August 28, 2023 19:22
.github/CODEOWNERS Outdated Show resolved Hide resolved
@AA-Turner AA-Turner changed the title ✨ Add PEP: Documentation Metadata in Typing PEP 727: Documentation Metadata in Typing Aug 28, 2023
pep-0755.rst Outdated Show resolved Hide resolved
@AA-Turner AA-Turner added the new-pep A new draft PEP submitted for initial review label Aug 28, 2023
@AA-Turner AA-Turner self-requested a review August 28, 2023 19:31
pep-0755.rst Outdated Show resolved Hide resolved
pep-0755.rst Outdated Show resolved Hide resolved
pep-0755.rst Outdated Show resolved Hide resolved
pep-0755.rst Outdated Show resolved Hide resolved
pep-0755.rst Outdated Show resolved Hide resolved
pep-0755.rst Outdated Show resolved Hide resolved
pep-0727.rst Outdated Show resolved Hide resolved
tiangolo and others added 2 commits August 28, 2023 21:47
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good! The comment about Sphinx isn't blocking for getting the PEP merged, but buy-in from Sphinx would make the PEP stronger.

@tiangolo
Copy link
Sponsor Contributor Author

Thanks a lot @JelleZijlstra! That was so fast. 🤯

I know very little about Sphinx, but if @AA-Turner could help me or point me in the right direction, I would definitely try to find a way to make it happen.

@JelleZijlstra JelleZijlstra merged commit c8e245d into python:main Aug 28, 2023
5 checks passed
@tiangolo tiangolo deleted the typing-doc branch August 28, 2023 19:58
@tiangolo
Copy link
Sponsor Contributor Author

Thanks for merging @JelleZijlstra!

I guess you can imagine my proud and happy smile right now (even if this never gets approved). I feel like a kid graduating or something. 😊

@PIG208
Copy link
Contributor

PIG208 commented Aug 29, 2023

Amazing @tiangolo! Maybe we can start a thread at https://discuss.python.org/c/peps/19 for community discussions (and to also fill the discussion-to field)?

@tiangolo
Copy link
Sponsor Contributor Author

Yes @PIG208! I was waiting to have it available in typing_extensions (I have the PR here: python/typing_extensions#277) as that is mentioned in the PEP, and it could be confusing if people tried it and didn't find it.

But now I'm not sure I should just start the thread right away... I'll wait for @JelleZijlstra's opinion and then I'll make the thread depending on that. 🤓

@JelleZijlstra
Copy link
Member

It's fine to open the discussion now. I'll take a look at the typing-extensions PR tonight and hopefully merge it and cut a release.

@tiangolo
Copy link
Sponsor Contributor Author

Great, thank you @JelleZijlstra!

I just created the discussion here: https://discuss.python.org/t/pep-727-documentation-metadata-in-typing/32566

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.

Hi @tiangolo, thanks for the PEP. I've reviewed in-line here. I'll open a PR to address some of the more editorial points of my review.

Thanks,
Adam

pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
pep-0727.rst Show resolved Hide resolved
@tiangolo
Copy link
Sponsor Contributor Author

Thanks a lot for the thorough review and sincere feedback @AA-Turner, in particular, thanks for giving all that feedback without any negative emotion (e.g. sarcastic criticism, derogatory comments, etc.). 🙇

I think it makes sense to merge #3316 first and then I'll work on anything else missing from your review.

Then I'll also work on the updates from other feedback, how other languages do, and a lot of Eric Traut's comments.

@AA-Turner AA-Turner mentioned this pull request Sep 1, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-pep A new draft PEP submitted for initial review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants