Skip to content

Improve closed PRs reopening corner cases#30

Merged
dimikot merged 1 commit intomainfrom
grok/dimikot/improve-closed-prs-reopening-corner-cases-to-main-c887
Apr 14, 2025
Merged

Improve closed PRs reopening corner cases#30
dimikot merged 1 commit intomainfrom
grok/dimikot/improve-closed-prs-reopening-corner-cases-to-main-c887

Conversation

@dimikot
Copy link
Owner

@dimikot dimikot commented Apr 14, 2025

Summary

GitHub is "too smart", so sometimes it marks the PRs as merged or as closed on its own, right after the PR head branch is force-pushed:

  1. If the head branch is force-pushed, and GitHub thinks that all the code is merged to the upstream already, it marks the PR as merged.
  2. If the head branch is force-pushed the way the PR has 0 commits in it, GitHub closes that PR and makes it unopenable.
  3. When the PR is closed anyhow, and then the head branch is force-pushed to some new commit hash, then GitHub refuses to reopen that PR (see Allow to reopen pull requests after a force push isaacs/github#361 for details).

We used to cover the case (1) (git-grok was just creating a new PR). Now we cover the case 2 (the same way as case 1) and case 3 (with a hack explained at isaacs/github#361).

How was this tested?

git grok

All those PRs were closed, then randomly reordered, and git-grok rerun:

CleanShot 2025-04-13 at 23 05 43@2x

PRs in the Stack

(The stack is managed by git-grok.)

@dimikot dimikot force-pushed the grok/dimikot/improve-closed-prs-reopening-corner-cases-to-main-c887 branch from c887e44 to e7fe862 Compare April 14, 2025 10:10
@dimikot dimikot added the git-grok-middle-pr This PR is in the middle of the stack. label Apr 14, 2025
dimikot added a commit that referenced this pull request Apr 14, 2025
## Summary

Adding an engine which is able to push multiple branches (commits)
atomically. For now, it still does most of the pushes sequentially, but
using the parallel engine (with commits list size 1).

The problem is that multi-branch push can only be used safely when no
commits reordering happened in the stack (otherwise, GitHub may mark
some of intermediate PRs as merged prematurely, and there is no way out
of it). That's why in the current PR, we do not utilize it much yet. It
will be treated in the next PRs.

## How was this tested?

CI.

## PRs in the Stack
- #31
- #30
- ➡ #29

(The stack is managed by
[git-grok](https://github.com/dimikot/git-grok).)
Base automatically changed from grok/dimikot/partially-switch-to-atomic-multi-branch-push-to-main-b4e5 to main April 14, 2025 10:25
@dimikot dimikot force-pushed the grok/dimikot/improve-closed-prs-reopening-corner-cases-to-main-c887 branch from e7fe862 to 583b632 Compare April 14, 2025 10:26
@dimikot dimikot removed the git-grok-middle-pr This PR is in the middle of the stack. label Apr 14, 2025
@dimikot dimikot merged commit 55419cd into main Apr 14, 2025
2 checks passed
@dimikot dimikot deleted the grok/dimikot/improve-closed-prs-reopening-corner-cases-to-main-c887 branch April 14, 2025 10:35
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.

1 participant