This message was deleted.

cc @Jacob Gold @David Bradford

Hi Joseph, Can you DM the PR this happened on and I can investigate?

I had something similar happen (don't remember the PR), where I enabled "merge when ready" while CI checks were still going, then thought better of it and clicked to disable "merge when ready". The UI was weird for a moment (can't remember how), and then Graphite merged the PR before CI checks were complete.

I chatted with Joseph offline. What was happening is that some of the CI checks were not marked as required. "merge when ready" will only wait for required CI checks to be successful before merging. If there are CI checks that are not required, "merge when ready" will not wait for them.

this was user error, but we thought our checks were required bc we had enabled checks required branch protection. but we had not explicitly selected all the checks in GH settings, so in reality all of our checks were optional 😞

ah gosh, same here 😕

I’d expect optional checks to complete tho - optionality only means it doesn’t matter if it fails.

fwiw, GitHub's "merge when ready" option behaves the same...

same how, in that it merges before optionals are complete? or that it merges once all checks are done, but ignores fail status of optionals?

@Jordan Harband we’re considering this as a user option setting. But thinking it through… if it’s going to merge regardless of the completed optional status (i.e. regardless if passing or failing), then we’re simply delaying merging. Is this the desired behavior? If so, can I get a better understanding of why you prefer waiting for optional checks to complete as opposed to merging immediately when all required checks pass?

for a number of reasons. one of them is that github shows how many checks were completed at merge time, so it look shoddy if it's less than 100%. another is that those optional checks are still important, and i still want them to pass, and once the PR merges, the branch will be auto-deleted, which means that those checks might fail - especially since the checkout step may not have run yet.

Got it, thanks for the additional context

those optional checks are still important, and i still want them to pass,

1. Do you want it to merge if the optional checks fail as long as they complete before the merge happens? 2. If not, are you able to set them as required instead and have it wait until it passes to merge?

1. yes. they're optional, which means i want them to run but it's ok if they fail

Got it, thanks! We’ll probably add an option to wait for all checks to complete before merging

sounds great. altho i think that should still be the default, since it's safer

Just ran into the same thing, was very surprised. It actually ended up in a very annoying state where a branch higher in the stack merged into one of the lower branches after that lower branch was merged, but before the lower branch was deleted from GitHub. So I've now had to play the revert/revert dance to pull that code back up and merge it to master manually. Quite frustrating tbh

"Merge when ready" was built to trigger as soon as the PR is allowed to merge (as someone mentioned above, it is at parity with GitHub automerge for unstacked PRs). Out of curiosity, why aren't these checks required if you expect them to pass before merging? I imagine to build what you want here, we'd need to build a more general "merge when X" system which might not be for a while, to be fully transparent... As for the upstack branches merging before downstack branches have been deleted issue — that shouldn't happen @Jonny O'Mahony — can you DM me which PRs were involved, and i'll look into it?

@Jacob Gold i don't expect them to pass, i expect them to complete, because a check being non-required doesn't mean it can be skipped, it just means a failure will be ignored. to be fair tho i don't use "merge when ready" (mostly because github still lacks fast-forward-merge)

For us, they are required, but we sometimes need to merge quickly to fix bugs etc. as we’re still very small and want to be able to ship / iterate quickly. Specifically in cases of reverts etc. We still the same suite on deployment so in those scenarios we’re fine with it. As a result, you can merge to our master without approval. I’ll DM you the PRs now Jacob, thanks 👍 EDIT: They are required by us semantically, but not by GH

for us it was just a GH misunderstanding. we had enabled require checks on all our repos, but it was unclear that we had to opt-in to each check in repo settings to actually make them required. thats fixed now 😓