Conversation
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.
mislav
left a comment
There was a problem hiding this comment.
The approach looks good! Left a suggestion about avoiding having to do changes to mergePullRequest() function and its payload
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"}, |
There was a problem hiding this comment.
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.
|
A related feature idea that you could consider adding @cristiand391 (no worries if not): if a regular |
Done! 🤘🏾
|
samcoe
left a comment
There was a problem hiding this comment.
This looks good to me, thanks for making the requested changes! 🙇
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
left a comment
There was a problem hiding this comment.
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.
This PR improves the current way
ghhandles an auto-merge by making it schedule an auto-merge only when themergeStateStatusfield isBLOCKED(otherwise it should just merge it).Ref. #3514 (comment)
Closes #3514
Fixes #3858