-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Comments
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. |
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. |
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.
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.
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.
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:
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. |
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 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. |
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 🤞 |
Uh oh!
There was an error while loading. Please reload this page.
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
text
)Technical requirements
pep-NNNN.rst
)PEP
,Title
,Author
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Requires
,Python-Version
andReplaces
filled out if appropriateSponsor
listed and approves if not authored by a core dev or PEP editorPEP
header updated accordingly.github/CODEOWNERS
for the PEPDiscussions-To
andPost-History
Accepting/rejecting a PEP:
Accepting/rejecting a PEP checklist
Discussions-To
threadStatus
changed toAccepted
Resolution
link points directly to SC/PEP Delegate formal acceptance/rejected postDiscussions-To
,Post-History` and
Python-Version`` complete and up to dateMarking a PEP Final
Marking a PEP Final checklist
(or third-party tools, for packaging/typing topic PEPs)
Status
changed toFinal
canonical-doc
directiveWhere 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.The text was updated successfully, but these errors were encountered: