Skip to content

PEP 731: C API Working Group Charter #3476

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

Merged
merged 21 commits into from
Oct 13, 2023
Merged

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Oct 11, 2023

This is ready for review by the PEP editors and the general public. All co-authors have signed off on the current text (except for Victor's clarification of "other languages", which I took the liberty to merge).


Basic requirements (all PEP Types)

  • Read and followed PEP 1 & PEP 12
  • File created from the latest PEP template
  • PEP has next available number, & set in filename (pep-NNNN.rst), PR title (PEP 123: <Title of PEP>) and PEP header
  • Title clearly, accurately and concisely describes the content in 79 characters or less
  • Core dev/PEP editor listed as Author or Sponsor, and formally confirmed their approval
  • Author, Status (Draft), Type and Created headers filled out correctly
  • PEP-Delegate, Topic, Requires and Replaces headers completed if appropriate [none appropriate for this PEP]
  • Required sections included
    • Abstract (first section)
    • Copyright (last section; exact wording from template required)
  • Code is well-formatted (PEP 7/PEP 8) and is in code blocks, with the right lexer names if non-Python
  • PEP builds with no warnings, pre-commit checks pass and content displays as intended in the rendered HTML
  • Authors/sponsor added to .github/CODEOWNERS for the PEP

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

@gvanrossum gvanrossum requested a review from a team as a code owner October 11, 2023 11:35
@gvanrossum gvanrossum marked this pull request as draft October 11, 2023 11:35
@gvanrossum gvanrossum requested review from vstinner, encukou, iritkatriel and zooba and removed request for a team October 11, 2023 11:35
@gvanrossum
Copy link
Member Author

gvanrossum commented Oct 11, 2023

I intend to use the following process.

  • Each co-author reviews and provides feedback (currently the Mandate section probably needs some real work).
  • I wait for each co-author to add a comment approving the PR.
  • I undraft the PR, and wait for feedback from the PEP editors team (e.g. PEP number).
  • Rename to assigned PEP number.
  • Once 1-2 of the PEP editors team (minus co-authors) approves, merge.
  • Open a Discourse thread to discuss the PEP.
  • Link the Discourse thread to Discussions-To and Post-History headers.
  • Once Discourse comments stop pouring in (but giving it at least a week), submit to SC tracker.
  • ...
  • Profit!

Copy link
Member Author

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks @zooba!

@encukou
Copy link
Member

encukou commented Oct 12, 2023

@encukou Could you suggest a specific edit to replace both the sentence above ("The WG's mandate is...") and the bullet points?

I see that's changed now, and I agree with the result!

Let's put the bullet points in the guidelines PEP: capi-workgroup/api-evolution#26

@gvanrossum
Copy link
Member Author

I think we're approaching convergence. Can all of you approve this version, or make specific suggestions? I'd like to tick boxes 1 and 2 above.

@gvanrossum
Copy link
Member Author

I think we're approaching convergence. Can all of you approve this version, or make specific suggestions? I'd like to tick boxes 1 and 2 above.

And box 3, obviously (which follows logically from box 2). But first we need a better sentence for the first sentence of the Abstract.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

Co-authored-by: Victor Stinner <[email protected]>
@gvanrossum gvanrossum marked this pull request as ready for review October 12, 2023 22:26
@gvanrossum gvanrossum requested a review from a team October 12, 2023 22:27
@hugovk
Copy link
Member

hugovk commented Oct 12, 2023

Please use PEP 731.


pip install -U pepotron
...pep next
Next available PEP: 731

https://github.com/hugovk/pepotron

@vstinner

This comment was marked as resolved.

@gvanrossum gvanrossum changed the title PEP 9999: C API Working Group Charter PEP 731: C API Working Group Charter Oct 12, 2023
@hugovk

This comment was marked as resolved.

@hugovk

This comment was marked as resolved.

@hugovk
Copy link
Member

hugovk commented Oct 12, 2023

I've added the checklist from https://github.com/python/peps/blob/main/.github/PULL_REQUEST_TEMPLATE/Add%20a%20new%20PEP.md to the top post, but not checked any off.

One thing needed is a .github/CODEOWNERS entry.

@vstinner
Copy link
Member

One thing needed is a .github/CODEOWNERS entry.

I understand that pep-0731.rst @gvanrossum @encukou @vstinner @zooba @iritkatriel should be added to .github/CODEOWNERS.

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.

It feels odd to see parts of PEP 729 immediately get reused here, but I'm happy that my proposal was helpful!

@hugovk hugovk added the new-pep A new draft PEP submitted for initial review label Oct 13, 2023
@gvanrossum
Copy link
Member Author

It feels odd to see parts of PEP 729 immediately get reused here, but I'm happy that my proposal was helpful!

It felt like a useful template for this type of committee-forming PEP. We're probably going to borrow it for the Docs Editorial Board too. Thanks for coming up with it! We should probably add Contributed-By: Jelle to the final merge commit.

@gvanrossum
Copy link
Member Author

PEP editors:

The template @hugovk linked to claims that Discussions-To and Post-History are required, but I was planning to post to Discourse after the PEP has been merged into the main branch. Am I mistaken? Most of the initial discussion happened at the Brno sprint, and we decided that the next step would be a PEP, which would serve as a starting point for discussions on Discourse. I plan to create the Discourse thread as soon as the PEP is merged, and update the PEP headers as soon as I have a URL for the Discourse thread.

The only TODO I have for myself before this can be merged is @JelleZijlstra's comment on line 23.

@hugovk
Copy link
Member

hugovk commented Oct 13, 2023

There's a bit of a catch-22: we can either merge the PEP without the fields, then post to discuss.python.org, and update the PEP file in another PR.

Or post to discuss.python.org first, then update the file.

@gvanrossum
Copy link
Member Author

There's a bit of a catch-22: we can either merge the PEP without the fields, then post to discuss.python.org, and update the PEP file in another PR.

Or post to discuss.python.org first, then update the file.

So what's the best practice? It seems the tests pass without those header fields.

@hugovk
Copy link
Member

hugovk commented Oct 13, 2023

I suggest: merge, post to discuss.python.org, then create a new PR.

This is a bit nicer, you can post a proper link in the discussion.

@Rosuav
Copy link
Contributor

Rosuav commented Oct 13, 2023

I suggest: merge, post to discuss.python.org, then create a new PR.

Agreed. Let's make them optional fields and permit new PEPs without them, and encourage that followup PR workflow.

@gvanrossum gvanrossum merged commit 32adf8f into python:main Oct 13, 2023
@gvanrossum gvanrossum deleted the capi-pep branch October 13, 2023 13:12
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.

8 participants