Skip to content

PR auto-merge improvements - #3706

Merged
mislav merged 10 commits into
cli:trunkfrom
cristiand391:improve-automerge
Jul 20, 2021
Merged

mislav merged 10 commits into
cli:trunkfrom
cristiand391:improve-automerge

Conversation

@cristiand391

@cristiand391 cristiand391 commented May 24, 2021 •

Copy link
Copy Markdown
Contributor

This PR improves the current way gh handles an auto-merge by making it schedule an auto-merge only when the mergeStateStatus field is BLOCKED (otherwise it should just merge it).

Ref. #3514 (comment)

Closes #3514
Fixes #3858

When passing `--auto` flag, only schedule an auto-merge if
the `mergeStateStatus` field is "BLOCKED".
This ensures that a PR will always be merged when passing `--auto` even
if it doesn't have required checks or if checks have already passed.
@cristiand391
cristiand391 marked this pull request as ready for review June 15, 2021 01:51

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach looks good! Left a suggestion about avoiding having to do changes to mergePullRequest() function and its payload

Comment thread pkg/cmd/pr/merge/merge.go Outdated
Comment thread pkg/cmd/pr/merge/merge.go Outdated
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeable", "headRepositoryOwner", "headRefName"},
Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeable", "mergeStateStatus", "headRepositoryOwner", "headRefName"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that adding this field to the query most likely makes the gh pr merge command unusable for GitHub Enterprise versions older than 2.20, since the field was only added then. Those GHE versions are not officially supported anymore, so I guess this is fine.

@mislav

mislav commented Jun 18, 2021

Copy link
Copy Markdown
Contributor

A related feature idea that you could consider adding @cristiand391 (no worries if not): if a regular gh pr merge (no --auto) fails because PR status is BLOCKED, perhaps the error message can suggest that you redo the invocation and add --auto to have the PR being merged as soon as it's unblocked.

@cristiand391

Copy link
Copy Markdown
Contributor Author

if a regular gh pr merge (no --auto) fails because PR status is BLOCKED, perhaps the error message can suggest that you redo the invocation and add --auto to have the PR being merged as soon as it's unblocked.

Done! 🤘🏾

automerge-error

@cristiand391
cristiand391 requested a review from mislav June 19, 2021 15:11
Comment thread pkg/cmd/pr/merge/merge.go Outdated
@samcoe
samcoe requested a review from vilmibm June 21, 2021 22:31

@samcoe samcoe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks for making the requested changes! 🙇

mislav added 2 commits July 20, 2021 18:45
Conditions prohibiting a regular merge: BLOCKED, BEHIND, DIRTY.

Conditions triggering a regular merge even if `--auto` was set: CLEAN,
HAS_HOOKS.

Note that UNKNOWN status does not trigger either of the conditions.

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I've adjusted the feature so that it's more conservative: especially when it comes to UNKNOWN status which is fairly common in my testing.

This status describes a state where the head branch is mergeable and
technically not blocked per base branch requirements, but it does have
non-passing checks.
@mislav
mislav enabled auto-merge July 20, 2021 17:44
@mislav
mislav disabled auto-merge July 20, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Seeing "Pull request is not in the correct state to enable auto-merge" with gh pr merge {x} --auto gh pr merge --auto command behaviour is confusing

4 participants