[llvm-dev] Enable Contributions Through Pull-request For LLVM

David Greene via llvm-dev llvm-dev at lists.llvm.org
Mon Nov 11 11:30:24 PST 2019


Philip Reames <listmail at philipreames.com> writes:

> Let's say I post a patch, and the (warranted) reviewer comment is that
> it should be split.  I push a couple new PRs, they get approved, and
> landed.  (All as new branches off master.)  Then, I have a stale branch
> (say it contains 2 commits) which needs rewritten.  I can of course do
> an interactive rebase, and a force push, but a) I've found interactive
> rebases to be very error prone, and b) I *strongly* prefer to never be
> in the habit of doing a force push.  The alternative is to close the
> original PR, and simply post a new one - which looses history of review. 

I would say that this is a case where a force push is not only ok, it's
actually desirable.  Every time we committed something via SVN we
essentially did a rebase and force push.  Currently with git we
essentially do a rebase and force push for anything landing in master
since we require fast-forward merges.

In your scenario, you responded to review, splitting off some smaller
commits.  Now you want to go back and get review of the updated patch.
But you have to update it in the context of those smaller commits you
already made to master.  This is exactly what rebase + force push is
for.  You're force pushing to your own fork, you are not rewriting
history others will use directly.

A lot of damage has been done by well-meaning people posting blog
articles with rules like "never rebase" and "never force push."  There's
a reason those operations exist and a proper way to use them.

                   -David


More information about the llvm-dev mailing list