-
Notifications
You must be signed in to change notification settings - Fork 214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
linear-history should enforce rules on rebase too #8500
Comments
Apparently mergify only runs on up to date PRs if Edit: we don't want to use Solution is likely to simulate a rebase, maybe only if the PR is up-to-date, before checking if there are any fixup commits left. |
Related to this issue, we should also enforce that no merge commits exist on |
refs: #8500 ## Description This PR does the following: - adds a new rule to mergify which forces it to rebase and autosquash the branch. - adds a condition of `linear-history` to the queuing rule for rebasing, so that only rebased PRs can be queued. - adds a new fixup commit ci check which is a required condition for merging **Note**: autosquash here refers to `--autosquash` flag used by the `git rebase` command. it is used to squash and combine `fixup!` commits. it does not squash all commits into a single commit before merging It fixes the following problems in #8500 - rebases the PR to remove all merge commits and `fixup!` commits - forces rebase on all PRs even if they are up to date with the master branch - enforce `linear-history` check on `automerge:rebase` enqueue to always enforce a linear-history before merging - the `rebase and autosquash` rule in mergify also fails if the PR cannot be rebased without conflicts. The failure indicates to the user that the PR must be rebased manually ### Examples of use: Testing rebase on PR that was up to date: frazarshad/mergify-experiements#2 Testing if a fixup with incorrect title stops the PR from merging: frazarshad#6 ### Fallback: In case the new action does not work as expected and does not auto-rebase, the user can still manually rebase in order to use the `automerge:rebase` action. All other `automerge` actions work as originally intended. In order to bypass the `no-fixup-commit` check, simply add a `bypass:linear-history` label to the PR ### Comparison report: This is a comparison made of the previous method for merging and the new method: https://drive.google.com/file/d/1kfIW-pVOORuYwBOQlxt-2cus0k9ynAfb/view?usp=drive_link ### Security Considerations ### Scaling Considerations ### Documentation Considerations ### Testing Considerations ### Upgrade Considerations
completed by #9794 |
Describe the bug
#8253 added a
linear-history
check which enforce that nofixup
or merge commits exists, to address #7768. The check is only enforced on merge PRs, assuming the rebase PRs would get autosquashed by mergify, or somehow always rebased. However if thefixup
commit doesn't have a base to fixup, mergify will happily proceed forward (I believe that's the default git behavior). Also if the PR is already up to date from a merge commit with the destination branch, the rebase will not be performed.To Reproduce
See #8481 and #8559
Expected behavior
No fixup or nested merge commits in history, regardless of automerge strategy
Additional context
We either need to get mergify to prevent merging in the rebase cases, or modify the
linear-history
check to run on rebase (likely has to simulate the interactive rebase first).The text was updated successfully, but these errors were encountered: