python / devguide

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

Improvements to Lifecycle of a pull request #1358

Open ziima opened 2 months ago

ziima commented 2 months ago

I write this issue as a feedback from Euro Python 2024 sprint to denote troubles I encountered as I contributed to cpython.

Describe the enhancement or feature you'd like Missing content in "Lifecycle of a pull request" https://devguide.python.org/getting-started/pull-request-lifecycle/

Describe alternatives you've considered None.

picnixz commented 2 months ago

My 2 cents: I don't like having gh-<number> in the commit message or in the branch because the issue logs become extremely noisy due to GitHub saying something like "XXX added a commit that referenced that issue YYY hours ago". Not sure if it's my GH extension though but I think the PR title should be sufficient in general.

However, the first and last points are worth mentioning (because otherwise the CI/CD is really unhappy).

Some precision: while I understand the "making good commits" section, this is usually fine if you only have a single commit or and likely don't modify more things afterwards. After a review, it is common to update typos or address reviews and I honestly don't think that adding gh-<number>: address review would give you a better commit message. What's important IMO is that you organize your commits in such a way that they are incrementally easy to review (I say this but my commits also fail to follow this rule) and don't force-push unless you forgot to locally squash multiple commits into one (like, two commits for fixing up a typo). But my experience tells me that you can easily have some surprises if you always rebase+force-push :D

For the branch, I think it's essentially for you to remember which branch you are working on. My local branches are sometimes named differently than my remote branches just for triaging them (I mean, if you have many "fix-bug-in-this-module" branches you don't necessarily remember which issue you are working on...)

JelleZijlstra commented 2 months ago

As a core developer merging PRs, I don't really care what you name your branch; it doesn't affect me.

The main way commit messages matter is that they affect the final commit message that will get used when I merge the PR. GitHub gives a text box pre-filled with the existing commits. I usually just remove all but the first one (since they're mostly going to be small fixes and cleanups), so it doesn't really matter to me what you put there.

erlend-aasland commented 2 months ago

We don't need a branch naming convention, since we do not create branches on the python/cpython repo; they should always be created on the developers fork, so you can chose whatever convention you like.

This information could be included in the devguide using a better wording.