ci: block merging PRs with fixup! commits #430
Conversation
|
:) |
niklasdewally
force-pushed
the
check-for-fixups-before-merging
branch
2 times, most recently
from
89a1727
to
2f24cb2
Compare
|
oh, this means other tests don't run? that's not ideal. |
other tests arn't running because this doesnt change any rust code - it shouldn't interfere with anything we already have |
niklasdewally
force-pushed
the
check-for-fixups-before-merging
branch
2 times, most recently
from
369a012
to
5047932
Compare
|
did you manage to figure out what the difference is between the matijs action and the 13rac1 one? |
|
@ozgurakgun a QOL thing I just found: if you add the
nothing, the former was just broken. |
Block any PRs with any commits called fixup!, amend!, squash! from being merged. As discussed in conjure-cp#425, force pushing to existing commits is annoying to code reviewers. Another approach is to do `git commit --fixup <commit>` when you need to respond to a code review comment on a particular commit, then run `git rebase --autosquash` before merging to get rid of these fixup commits. (https://rietta.com/blog/git-rebase-autosquash-code-reviews/) This way, code reviewers can see the code review changes to the PR in a linear order, but still keep a proper git history.
niklasdewally
force-pushed
the
check-for-fixups-before-merging
branch
from
5047932
to
a008c21
Compare
|
is that an unfinished sentence in your last comment? |
Oops: I was messaging in the other PR where we were talking about this at the same time and got myself mixed up :( |
Add guidance to not force push to PRs, as discussed in conjure-cp#430 and conjure-cp#425. This new guidance roughly follows how LLVM does stuff: https://llvm.org/docs//GitHub.html#updating-pull-requests
Add guidance to not force push to PRs, as discussed in conjure-cp#430 and conjure-cp#425. This new guidance roughly follows how LLVM does stuff: https://llvm.org/docs//GitHub.html#updating-pull-requests
Block any PRs with any commits called fixup!, amend!, squash! from being merged.
As discussed in #425, force pushing to existing commits is annoying to code reviewers. Another approach is to do
git commit --fixup <commit>when you need to respond to a code review comment on a particular commit, then rungit rebase --autosquashbefore merging to get rid of these fixup commits. (https://rietta.com/blog/git-rebase-autosquash-code-reviews/)This way, code reviewers can see the code review changes to the PR in a linear order, but still keep a proper git history.