Hacker News Reader

Comment by ctrlmeta | Hacker News Reader

Can someone explain what we are looking at? Perhaps a simple point by point explanation of what was done, what the expected behavior is and what we observe instead?

Do I understand correctly that these are the events that happened:

* septicmk forked v6d-io/v6d to septicmk/v6d

* septicmk created a pull request from septicmk/v6d:main to v6d-io/v6d:main

* septicmk merged sighingnow's commit 61f3741 to septicmk/v6d:main (perhaps while updating their fork?)

* GitHub says septicmk merged commit 61f3741 to septicmk/v6d:main?

If so is that not right? They did merge a commit in their fork at septicmk/v6d:main, didn't they?

Does GitHub say anywhere that they merged commit to v6d-io/v6d:main? That would be very interesting and a real issue!

I agree the big "Merged" button on top of the pull request is very misleading, perhaps a UX bug! But no unauthorized merge actually happened here, did it?

View on HN

What happened here (if I understand right) is:

* septicmk forked v6d-io/v6d to septicmk/v6d

* septicmk added commits to septicmk/v6d:main

* septicmk created a pull request from septicmk/v6d:main to v6d-io/v6d:main

* septicmk force pushed septicmk/v6d:main with the head commit of v6d-io/v6d:main

* Github closed the pull request and displayed a "merge" notification

The UX issue is that septicmk did not have write permission to v6d-io and did not actually merge any commit into it, but GitHub says "septicmk merged commit 61f3741 into v6d-io:main". Commit 61f3741 was authored by sighingnow and was already on v6d-io:main.

So in this case no authorized merge actually happened, the PR was just updated to have no difference from the actual repository's main branch, but GitHub makes it seem like a merge happened.

GitLab team member here. It seems related to the problem that you cannot open an empty PR https://stackoverflow.com/questions/46577500/why-cant-i-crea... and force overriding an old branch base is a workaround to land at an empty diff. Another workaround could be git commit —-allow-empty, making the GitHub UI think that no changes between branches is equal to the merged state.

The behavior is not reproducible in GitLab, where empty merge requests can be created, and remain in the open state, also with zero changes at some later point. Added in GitLab 9.2 as part of the issue to new branch and wip/draft merge request flow: https://gitlab.com/gitlab-org/gitlab-foss/-/issues/28558

Allowing "zero changes" state for a merge request does make sense, as during the development workflow developers may doing some rebase & force push work and they may forget commit changes after reset to main and before push to the pull requests.

Closing the "zero changes" merge request have confused in my previous experience when I continue to push re-added commits to the original pull request branch.

I think that Gitlab behaviour makes more intuitive sense. Thanks for chiming in.

Looks like they did a force-push that replaced the PR's from-branch (septicmk/v6d:main) with the state of the target branch. This effectively removes all commits from the PR, leaving the PR with zero remaining commits. All of the zero commits on the PR are fully merged into v6d-io/v6d:main, thus marking the PR as merged (technically correct, but confusing).

So in effect, unauthorized merges (PR looking merged; email notifications being sent) are possible if they are "zero-commit-merges" that don't actually change any state in the git repository.

indeed, I could reproduce it, just force push `master` into a PR, github UI will close the PR, marked it as merged, delete the source branch ... but of course nothing happens in the git repo :/

The "zero-commit-merges" should be treated as a "Closed" event better than being treated as a "Merged" event.

From Git's perspective, the two "forks" share the same HEAD commit, e.g. they are merged. That is what a merge does. A branch is a name that exists independently of the commit and points to it, so the commit history is not actually a property of the branch, despite what Git UIs tell you.

Thus, in the underlying data structure behind the GitHub interface, there really isn't an "event" here to identify. The PR branch points to the same commit as the base branch, therefore both branches are in the state of existing as "merged" with one another.

So GitHub would have to track changes to the PR branch that result in this state separately from the existing "merge from GUI" and "push PR branch to master" changes, which I could imagine is fraught with edge cases that could result in what you consider a merge event ending up as a closed event.

GitHub automatically marks a PR as merged if its HEAD commit hash matches the BASE commit hash. Usually it happens when the PR code goes into the main branch, but it can also happen if the PR branch is forcibly reset to match the main branch. The bug here is that it closes the discussion prematurely (a discussion on a zero diff, at that specific moment in time). There's no unauthorized source code modification, as you noted. Awkward but meh.

Reply on news.ycombinator.com