8000 Have straightforward checklists for adding, accepting/rejecting and marking a PEP Final · Issue #2937 · python/peps · GitHub
[go: up one dir, main page]

Skip to content

Have straightforward checklists for adding, accepting/rejecting and marking a PEP Final #2937

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

Closed
25 tasks
CAM-Gerlach opened this issue Dec 20, 2022 · 7 comments
Closed
25 tasks
Assignees
Labels
meta Related to the repo itself and its processes

Comments

@CAM-Gerlach
Copy link
Member
CAM-Gerlach commented Dec 20, 2022

As discussed in the December 2022 Python Docs Community meeting, it seems like it would be a good idea to have a straightforward checklist concisely listing what needs to happen when a new PEP is added, when a PEP is approved/rejected, and when it is marked Final. This way, it is clear to both PEP authors and editors exactly the steps needed, both so they get applied consistently and they avoid having to dig through PEP 1, 12, the template, the devguide, the readme/contributing guide, and/or old PRs to figure out exactly what they need to do.

First, we should make sure we're clear on what we want in the checklist, and then we should decide how best to make them easily accessible (and ideally, checkable) to authors (and PEP editors). Therefore, for starters, I propose the following content (based on the relevant sections in PEP 1, PEP 12 and elsewhere):

Proposed content

Adding a new PEP

Adding a new PEP checklist

Content requirements

  • PEP topic discussed in a suitable venue and general agreement that a PEP is appropriate
  • Content is reasonably sound, complete and makes at least basic technical sense
  • Abstract and Copyright sections completed, with the required wording in the latter
  • Suggested sections included (unless not applicable)
  • Title clearly, accurately and concisely describes the content
  • Prose is intelligible and generally free of basic grammar and syntax errors, at least to the degree they significantly detract from understanding
  • Code is in code blocks, well-formatted (PEP 7/PEP 8) and has the right lexer name if non-Python (or text)
  • Any project stated in the PEP as supporting/endorsing benefiting from it confirms such

Technical requirements

  • Filename in the correct format (pep-NNNN.rst)
  • PEP, Title, Author, Status (Draft), Type and Created headers filled out correctly
  • PEP-Delegate, Requires, Python-Version and Replaces filled out if appropriate
  • Sponsor listed and approves if not authored by a core dev or PEP editor
  • PEP builds with no warnings, CI checks pass and content displays as intended in the rendered preview
  • PEP number assigned by the PEP editors and filename & PEP header updated accordingly
  • Sponsor/author(s) that are core devs/PEP editors added to .github/CODEOWNERS for the PEP
  • Right before or after initial merging, PEP discussion thread created and linked to in Discussions-To and Post-History

Accepting/rejecting a PEP:

Accepting/rejecting a PEP checklist
  • SC/PEP Delegate has formally accepted/rejected the PEP and posted to the Discussions-To thread
  • Status changed to Accepted
  • Resolution link points directly to SC/PEP Delegate formal acceptance/rejected post
  • Discussions-To, Post-History` and Python-Version`` complete and up to date

Marking a PEP Final

Marking a PEP Final checklist
  • Specification has been implemented in a released stable CPython version
    (or third-party tools, for packaging/typing topic PEPs)
  • Status changed to Final
  • Canonical docs/spec moved to an appropriate place, and linked with a canonical-doc directive
  • PEP headers and content updated with any substantial changes during the implementation phase

Where to put the checklist

Ideally, it is most visible and easy to find for all involved if its right in the relevant PR body template as opposed to having to manually dig through PEP 1, PEP 12, the Readme, the Contributing Guide or the devguide, and allows the author, reviewers and PEP editors to actually check things off as they are completed/confirmed, allowing everyone keep track of what's done and what's needed.

The one obstacle is while GitHub supports multiple templates, but but there's no actual "chooser" UI yet for it like for issues—the template name has to be manually input via a query parameter in the new PR URL.

We could add put each checklist into a separate sub-template and then expose links to each with the appropriate query param set, which would avoid belaboring the main template with them. However, we run into the issue that both new authors and old hands alike are unlikely to find these links if we bury them in one of the existing places, as newer/occasional contributors may not know where (or remember to) look where to find them, and more experienced ones are unlikely to know something has changed, and either way just create the PR via the normal, expected way. We could also put a big reminder in the default PEP template, but it still could be easily missed, whether by a new contributor not knowing what to do, or a veteran one not bothering to look. Either way, it defeats a lot of the benefit of the checklist if it isn't easy to use by default and is used consistently.

Instead, what I'd suggest is having each checklist as subsections in the default PR template, and tell the user to just delete the checklists that aren't relevant (or all of them, with a Ctrl-A Del), as its a lot less work to delete sections (or all of it) then go find the right checklist and then copy and paste it in. Then, in case they do miss it, they'll see it when they submit so they'll know for the future, and either they or we can easily delete the not-relevant ones from their PR. I'm not sure what else would be that effective since it'll be too easy to miss otherwise, considering how many experienced devs don't use the actual PEP template (as opposed to copying an old outdated PEP, or just winging it), read the PR template admonition about prefixing their PR titles with the PEP number, etc. If and when GitHub added a PR chooser, we could switch to using that once implemented.

@CAM-Gerlach CAM-Gerlach added the meta Related to the repo itself and its processes label Dec 20, 2022
@CAM-Gerlach CAM-Gerlach self-assigned this Dec 20, 2022
@CAM-Gerlach CAM-Gerlach changed the title Have a simple checklist for adding, accepting/rejecting and marking a PEP Final Have streightforward checklists for adding, accepting/rejecting and marking a PEP Final Dec 20, 2022
@Rosuav
Copy link
Contributor
Rosuav commented Dec 20, 2022

Nice. It may be worth enumerating some of the "suitable venues" for discussion, including the mailing lists, Discourse, and whatever discussion places are used for the special interest groups.

@encukou
Copy link
Member
encukou commented Dec 20, 2022

To me these look like review checklists. They should be in a place where the reviewer (a PEP editor, usually) can find them, and copy them to their review comment. Devguide would be a good place.

Making sure PEP authors see and follow these is secondary. I think most authors will either find them helpful and start using them (after they get a review with one), or they'd ignore/delete the checklist in the initial post anyway.

@CAM-Gerlach
Copy link
Member Author
CAM-Gerlach commented Dec 20, 2022

Nice. It may be worth enumerating some of the "suitable venues" for discussion, including the mailing lists, Discourse, and whatever discussion places are used for the special interest groups.

Good point. Instead of further belaboring the checklist, I've instead linked the section in PEP 1 that describes them in detail for any readers interested in more about that.

To me these look like review checklists. They should be in a place where the reviewer (a PEP editor, usually) can find them, and copy them to their review comment.

That's true, but they also are intended to serve the equa 8000 lly important purpose of giving the PEP author a concise but point by point list of things they need to make sure they have for their PEP to be added, accepted/rejected or marked Final, all in one place—right in the PR—as opposed to having to trawl through PEP 1, 12, the readme/contributing guide, or the devguide looking for the relevant bits. This makes things easier and less uncertain for authors, lightens load on reviewers, and reduces back and forth for both, ensures everyone is on the same page and knows what's expected up front.

Devguide would be a good place.

Perhaps it could be eventually, if we do decide on and implement moving the canonical PEP 12 content there. However right now, there is no PEP page or section on the devguide, and it instead defers to PEP 1 and PEP 12 for that—which contain the author- and PEP editor-facing sections on which the checklist is based. So I don't think we should stick the checklists there at least right now before moving the other guidance, and expecting users to go to a whole other website/repo to find it doesn't really solve the main visibility/ease of access issue.

Making sure PEP authors see and follow these is secondary. I think most authors will either find them helpful and start using them (after they get a review with one), or they'd ignore/delete the checklist in the initial post anyway.

Including a checklist in the PR template, if there is one, is a pretty common convention in other repos, whereas I've much more rarely seen them in reviewer comments, at least for standard repo-wide checklists. Putting it in the OP:

  • Ensures it will be visible to the author pre-submission and to everyone right after (rather than only if and when a PEP editor sees it at some point before merge)
  • Is easier to find up top for everyone rather then buried in a specific review comment in the discussion
  • Allows both authors and other editors to collaborate on it, as opposed to in one specific reviewer's comment
  • Is less work being included automatically (potentially only deleting the other checklists) than manually finding it and copying it in (or each PEP editor configuring and using them as a canned responses, for those familiar with that feature)

At worst, if the author did delete it all mistakenly, a PEP editor (or any org member) could simply add it back as easily as adding it in their own comment. And if the burden of deleting the other non-relevant checklists, or the whole thing, was judged to be too much, the fallback approach as discussed would be using separate templates for each checklist, and prominently linking them in the default PR template and elsewhere for authors to use if relevant. That wouldn't be ideal, since it would be a good deal easier to miss and perhaps more work for authors to find, choose and navigate to the links (or elsewhere) than just delete the irrelevant sections from the default template, but it at least would be a strict upgrade over the status quo with minimal change to the existing template.

@hugovk hugovk changed the title Have streightforward checklists for adding, accepting/rejecting and marking a PEP Final Have straightforward checklists for adding, accepting/rejecting and marking a PEP Final Dec 20, 2022
@CAM-Gerlach
Copy link
Member Author
CAM-Gerlach commented Dec 24, 2022

I had thought up another idea...we could have a GH Actions bot to automatically insert the correct checklist in the OP based on whether it creates a new PEP or it modifies the Status header to the Approved/Rejected or Final states, like Bedevere does. That would avoid people having to either having know about, find and follow the special parameterized URLs for the templates, or manually delete the irrelevant checklists from the master template, though it does means that the checklist is not visible until after the PR is already submitted, which is not ideal.

However, if GitHub implements a template chooser UI for PR templates, which it seems reasonable to expect that they might soon, given I'm not sure how trivial it would be to implement the logic for that (especially the check logic) and its still not ideal, maybe it is worth just going with the separate templates option with parameterized URLs (called out in the main PR template and elsewhere) until that is deployed, which would be a strict (if not as easily accessible) upgrade on the status quo while avoiding any disruption/extra work for existing contributors, allowing a smooth transition to such a feature with minimal wasted effort.

@CAM-Gerlach
Copy link
Member Author

So I hear that we might have a special flag enabled for this repo that enables a new experimental alpha feature to provide a PR chooser UI :) I opened a PR which implements these checklists in optional templates and will hopefully trigger the new functionality: #2956 🤞

AA68

@mahamad1234

This comment was marked as spam.

@hugovk
Copy link
Member
hugovk commented Oct 12, 2023

Templates were added in #2956 and documented in #3474 🚀

@hugovk hugovk closed this as completed Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Related to the repo itself and its processes
Projects
None yet
Development

No branches or pull requests

5 participants
0