[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