[PATCH] D150594: docs/GitHub: Add note about force-pushing when rebasing on main

Wed May 17 20:27:32 PDT 2023

mehdi_amini added inline comments.

================
Comment at: llvm/docs/GitHub.rst:40
+track the context of previous review comments.  However, if you are rebasing
+your changes on the latest main branch, then your only option is to force push.

----------------
jhenderson wrote:
> mehdi_amini wrote:
> > Is this an opportunity to squash? Does GitHub preserve better the history if we force-push on rebase but don't squash?
> > 
> > The issue with this way of adding "fix up" commits is that a conflict during rebase may lead to as many redundant manual conflict resolution as there are "fix up" commits.
> Force pushes being unavoidable when doing rebases is one of the reasons I have a dislike of reviewing on GitHub, due to the issues it has.
> 
> Regarding the additional sentence itself, I'm fine with it. I don't know of a way that will improve comment history more than anything else (squashing probably is safe, and I have no objection to encouraging it as per @mehdi_amini's comment). That being said, I haven't done any rigorous testing.
Technically one would want to squash first, and rebase after! (to avoid the multiple conflicts resolution)

There is also the option of merging main back into the user branch instead of force-pushing: why not this?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150594/new/

https://reviews.llvm.org/D150594