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

[James Henderson via Phabricator via llvm-commits]

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:
> 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?
> There is also the option of merging main back into the user branch instead of force-pushing: why not this?

Doesn't that require a merge commit? I'm pretty sure we don't want those in our main branch history, and I don't know how that then plays with the squash & merge button at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150594