How to cleanly merge main into a PR branch?

I want to help an author of python/cpython#28245 by adding What’s New and documentation bits. To do this I want to open my PR against the author’s branch in their fork.

Since Doc/whatsnew/3.11.rst was modified in an area of my change, I need to actualize the author’s branch to main first. To achieve this, I took my cloned fork and ran:

$ git remote add pacien https://github.com/pacien/cpython.git
$ git fetch pacien
$ git checkout -b gh-28245 pacien/bpo-45141-mailcap-files-param
$ git merge origin/main
$ <created my own commit here>
$ git push origin gh-28245  # origin is git@github.com:arhadthedev/cpython.git
$ git log --oneline --graph
* 5c5d80577a (HEAD -> gh-28245, origin/gh-28245) Add user-visible notification on the change
*   8cd35c81b1 Merge remote-tracking branch 'origin/main' into gh-28245
|\
| * 23dcea5de7 (origin/main, origin/HEAD, main) bpo-40059: Fix installation of tomllib (GH-31784)
| * 02fbaf4887 bpo-46245: Add optional parameter dir_fd in shutil.rmtree() (GH-30365)
| * 5eb03b1b51 Fix 3.11 what's new formatting (GH-31763)
| * d1777515f9 bpo-45138: Expand traced SQL statements in `sqlite3` trace callback (GH-28240)
| * b33a1ae703 Docstring: replace pysqlite with sqlite3 (GH-31758)

However, a PR creation/diff page (Comparing pacien:bpo-45141-mailcap-files-param...arhadthedev:gh-28245 · pacien/cpython · GitHub)
shows that the merge went wrong and acceptance of my PR will display “arhadthedev added 1718 commits N minutes ago” in discussion of the original PR with invocation of many code owners.

What do I need to do to create a proper, transparent merge that GitHub shows as a single merge commit not reflected in “Files changed” tab?

Well, that’s exactly what your PR will do, relative to the author’s feature branch. The PR source is your branch containing both your new changes, and the merge to main bringing in all the other commits, while the target is the author’s feature branch, which doesn’t have those commits, not the main branch on the CPython upstream (which I presume you’ve just synced your origin/main branch from, though it is not shown above), which does.

Therefore, GitHub is correctly showing that your PR will add all of those commits to the author’s feature branch, the same as if you merged, say, a feature branch to a develop branch, and then merged develop to main—otherwise, the PR would show nothing.

Personally, for cases like these PRs don’t tend to be good tools for updating branches like this. In addition, I (and the various projects I work on) always rebase instead of merge to update an unmerged feature branch to the latest main branch, as the commit history is much cleaner and simpler to follow, rebase -i is much less troublesome, and it is much easier to fix Git mistakes or do cleanup later.

If the author is still active (and thus able to merge your PRs), I suggest just asking them to perform the rebase (or merge) themeslves, or giving you access to do so if they can’t. However, if you have to do this and can’t just make a new PR against upstream, you could at least make two separate PRs against the author’s branch: one with the merge and one with the new changes, so they can be reviewed independently. Also, using the commit view of the files pane allows reviewers to see what you changed in your one commit (though it is not as useful as being able to use the full files view).

I use git merge --no-ff main, which works well with GitHub PRs (contrary to rebases). In my experience, it is often easier to resolve conflicts using merge than with a rebase of a long lived branch.

If you try to open your PR against main in the CPython repo (instead of against the original author’s fork), you’ll see that it all looks ok; GitHub shows three commits:

  1. the original commit from GH-28245
  2. sync with main (your merge)
  3. your change
1 Like

Also, since we always merge to main using squash merges, it doesn’t matter how the history looks in the PR. Whatever it takes to make the review smooth (which generally means don’t ever force push) is best, and then (the core dev should) include enough info in the commit message that the PR history isn’t needed.

3 Likes