devguide icon indicating copy to clipboard operation
devguide copied to clipboard

Make it clearer that "automerge" exists and is preferred

Open Mariatta opened this issue 6 years ago • 5 comments

And provide better instructions about how to merge manually.

One thing that people don't realize is that the second PR number (in backport pull request) is autogenerated by GitHub, and had to be removed manually by hand before we click "Squash and Merge" button.

If we don't want to have to remember to remove that manually, then utilize the automerge feature.

As a reminder, automerge will be triggered if:

  1. In case of miss-islington's own PR, if the PR was approved by a core dev (has awaiting merge label), tests all passing, CLA signed, and no do-not-merge label.
  2. In case it is not miss-islington's own PR: same as the above, with addition of automerge label

Mariatta avatar Aug 02 '19 19:08 Mariatta

I wonder if the right place for this content is https://devguide.python.org/committing/ or if there is a need for a dedicated page which helps core devs understand the features of the bots?

There already is #520 to make the above page simpler, but I feel more text on that page will just be invisible. But perhaps when it is re-written to be a more of a step-by-step guide, it can delegate to specific sections of this new page dedicated to bots when pointing out the workflow.

maxking avatar Aug 11 '19 21:08 maxking

Automerge is great for backports, but is is wretched for original merges as the default title and message are usually wrong. Edits to title are ignored. I am not sure about messages, but is is wrong if the original PR had multiple commits and if there are trivial edits to the patch or multiple edits by one person.

terryjreedy avatar Aug 09 '22 16:08 terryjreedy

Automerge is great for backports, but is is wretched for original merges as the default title and message are usually wrong. Edits to title are ignored. I am not sure about messages, but is is wrong if the original PR had multiple commits and if there are trivial edits to the patch or multiple edits by one person.

This is not quite correct. There are two scenarios to consider, and they are confusingly handled very differently by GitHub:

Scenario Commit title Commit message
PRs with one commit The title of the commit message for the single commit, followed by the pull request number The body text of the commit message for the single commit
PRs with multiple commits The pull request title, followed by the pull request number A list of the commit messages for all of the squashed commits, in date order

For PRs with multiple commits, you can edit the PR description (the "OP" of the PR) and the PR title before you apply the automerge label. It works well.

See also the GitHub Docs regarding this behaviour.

erlend-aasland avatar Aug 09 '22 16:08 erlend-aasland

Automerge is great for backports

For backports I generally leave an "approve" review (after double-checking that the backport is indeed correct), and that is sufficient to trigger auto merging (at least for backports created by miss-islington).

There are two scenarios to consider, and they are confusingly handled very differently by GitHub:

There is also this setting: image

ezio-melotti avatar Aug 11 '22 02:08 ezio-melotti

For backports I generally leave an "approve" review (after double-checking that the backport is indeed correct), and that is sufficient to trigger auto merging (at least for backports created by miss-islington).

Ditto. By the way, do we mention the decisions GitHub takes when producing the squashed commit in the devguide? If not, we should; a lot of devs are surprised by this behaviour. It created a lot of headaches for me before I found out, at least.

erlend-aasland avatar Aug 11 '22 07:08 erlend-aasland