python / devguide

The Python developer's guide
https://devguide.python.org/
Creative Commons Zero v1.0 Universal
1.86k stars 779 forks source link

Squash/merge etc for accepting a pull request #1445

Open nedbat opened 1 week ago

nedbat commented 1 week ago

Describe the bug

Maybe I missed it somewhere, but I couldn't find a clear description of how to accept a pull request. Squash instead of merge, right? And when squashing, don't leave all the individual commit messages in the message (though, why not?)

skirpichev commented 1 week ago

Not sure if this is a good idea in general (some core devs seems using it, some apparently - not), but I'm trying to sync in my PR's description with actual content (following review process, etc). In this way, it could serve as a draft for the commit message.

erlend-aasland commented 1 week ago

I think squash is the only option enabled. Regarding commit messages, I was under the impression that we already had some rudimentary instructions on how to word them. If we don't, we definitely should add some!

And when squashing, don't leave all the individual commit messages in the message (though, why not?)

A branch with lots of small commits (often work-in-progress amendments) would pollute the resulting commit message intensely, and end up as an extremely verbose commit message that could consume a screenful or two.

Commit messages should (IMO) be counted as documentation; a succinct description of the intent and effects of the change should be a minimum requirement

hugovk commented 1 week ago

Yes, squash merge, it's the only option enabled:

image

I think removing the individual commit messages is a good idea, because usually there's not been much if any editorial care put into them. Partly because we squash anyway, we might have fixup commits, or "Apply changes from review", or other almost throwaway commit titles later in review.

If someone has written a useful description in their commit, we can leave this in place. This is rare though, and I nearly always look up commit -> PR (and sometimes -> issue) when I want more info and context. (And I use the Refined GitHub extension on desktop which auto-clears commit descriptions.)

nedbat commented 1 week ago

BTW, in this repo (devguide), "Rebase and merge" is enabled along with "Squash and merge."