Skip to content
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

Make it clearer that "automerge" exists and is preferred #518

Closed
Mariatta opened this issue Aug 2, 2019 · 5 comments · Fixed by #930
Closed

Make it clearer that "automerge" exists and is preferred #518

Mariatta opened this issue Aug 2, 2019 · 5 comments · Fixed by #930

Comments

@Mariatta
Copy link
Member

Mariatta commented Aug 2, 2019

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
@maxking
Copy link
Contributor

maxking commented Aug 11, 2019

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.

@terryjreedy
Copy link
Member

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.

@erlend-aasland
Copy link
Contributor

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.

@ezio-melotti
Copy link
Member

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

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Aug 11, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants