Over in this comment@CAM-Gerlach suggests turning on “Require conversations resolution before merging”, which I think makes sense for the PEPs repository. Let’s leave the question about python/cpython alone for now.
Reviewing the branch protection rules, I’d also suggest turning on “Require status checks to pass before merging”, but I think the rest we can leave off.
I’m a repo admin so I can turn them on, but I wanted to see if there are any objections first. Take a few days. Or maybe I’ll just turn them on and see if anyone complains.
Yeah, that would be a good idea (and have advocated for it in the past) since it will prevent commits from being directly pushed to the live PEPs site that break something for everyone (while still allowing authors full flexibility to change their own PEPs autonomously, so long as the build passes).
We could also enable Require a linear history, since we only have squash merges enabled anyway, and it would avoid someone accidentally pushing a merge commit to main, which would pretty much always be an unintended mistake.
Finally, the other non-branch protection setting I’d strongly suggest enabling is the new Default to pull request title and commit details option for the squash merge commit message (in case it isn’t on already, which it might be), which uses the PR title as the commit summary, but also lists the individual commit messages as bullets below that. This would resolve the longstanding issue (that a number of folks have raised issues with that the often much less descriptive/carefully worded first commit message gets used instead of the typically much better PR title, even if the latter is updated by the original author, a PEP editor or another core reviewer, while throwing away the other commit messages.
For reference, this was already implemented on the CPython repo, where previously e.g. on python/cpython#97618 had a squashed commit message of the rather uninformative “Fixup policy docs”, which happened to be the first commit on the PR branch, rather than the far more useful “Fix asycio policy docs about per interpreter semantics” PR title that had been set (and committers would generally expect to be used). This is particularly a problem on mobile where, so I hear, it is difficult or perhaps even impossible to change the final commit message.
It’s not clear to me if «commit details» means the list of all commit messages, which is always a pain to remember to delete. We have many commits with overlong repetitive messages on the main branch!
As mentioned, it is a bulleted list of the individual feature branch commit messages that gets put into the commit’s extended description. Out of habit (especially coming from projects that historically haven’t really used squash merges), I always make sure to make mine descriptive and non-duplicative even if they are going to be squashed, so they are a useful both for reviewers and as a bulleted summary description of what the squashed commit contains, as well as cull any ones that aren’t such when merging others’ PRs. It may require a bit of touch up sometimes, but at least IMO its a lot better than simply throwing away what can be quite useful information, and it only shows up when clicking the ... on a commit message anyway so it doesn’t really get in the way otherwise.
On the main PEPs branch, I have enabled Require status checks to pass before merging but not Require branches to be up to date before merging. I’ve also enabled Require conversation resolution before merging and Require linear history.
Default to pull request title and commit details option for the squash merge commit message (in case it isn’t on already, which it might be)
It already was!
Please follow up here if you encounter any problems.
Hey, quick tip—I just found out and confirmed you can use Refined GitHub’s clear-pr-merge-commit-message option to automatically clear the commit description field of everything but co-authors, if you prefer, which gives you the exactly same result as the PR title option (without the “commit details”) in the repo settings, just settable on your end.
