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

[Fedor Sergeev via llvm-dev]

On 11/11/19 10:30 PM, David Greene via llvm-dev wrote:
> 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.
The point is that while requiring rebase we do not require the force 
push into LLVM master, and
should not require doing that since force push *is* dangerous when you 
consider pushing into
public repository/branch.

(and no, SVN workflow is not force-push, it is auto-rebase-on-push instead)

I understand why Philip is nervous about having to even type the force 
push, since fingers can be
tricky to control sometimes (at least for me...).

I can see a straightforward solution for that problem though - just 
create an alias that encodes that force push, e.g.
   git update-pr pr1234
(where alias.update-pr is something like:
   'pf() { push --force MyLLVMGihub HEAD:$1;}; pf'
)

This way you free yourself from typing this --force thing manually and 
avoid accidental
destructive pushes into a public repo.

regards,
   Fedor.

>
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev