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

Danila Malyutin via llvm-dev llvm-dev at lists.llvm.org
Tue Nov 12 07:07:32 PST 2019


Aren't force-pushes disabled for the llvm master branch? If not, they should be.

--
Danila

> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Fedor
> Sergeev via llvm-dev
> Sent: Tuesday, November 12, 2019 13:28
> To: llvm-dev at lists.llvm.org
> Subject: Re: [llvm-dev] Enable Contributions Through Pull-request For LLVM
> 
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cg
> > i-2Dbin_mailman_listinfo_llvm-
> 2Ddev&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&
> > r=YgdxWMcdqQPlU9EdetI-
> xI79G7ouw9_Us0dFsZnFQYU&m=zgNhcK71xx2QMccUrPkIy3
> > jXQPJ-Hua6Jf3dhg9eRUQ&s=_69YJ6MQ0vj06r49okFg-
> DHCZvNP3H851IrvMCZaFBw&e=
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-
> 2Dbin_mailman_listinfo_llvm-
> 2Ddev&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=YgdxWMcdqQPlU9Ede
> tI-xI79G7ouw9_Us0dFsZnFQYU&m=zgNhcK71xx2QMccUrPkIy3jXQPJ-
> Hua6Jf3dhg9eRUQ&s=_69YJ6MQ0vj06r49okFg-
> DHCZvNP3H851IrvMCZaFBw&e=


More information about the llvm-dev mailing list