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

Wed May 17 00:34:30 PDT 2023

jhenderson 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.

----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150594