Skip to content
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

Tweak mergify config #151

Merged
merged 3 commits into from Jun 4, 2020
Merged

Conversation

mmlb
Copy link
Contributor

@mmlb mmlb commented Jun 4, 2020

This PR will change the merge strategy back to merge so that we get a lovely commit object in history to have a visual grouping of commits by PR. It also minimizes the config by leaning on defaults and the branch protections already enforced by GitHub. Finally, the last commit will ensure that all reviewers feedback have been addressed and not discarded by possibly unrelated approvals.

Rebase will rebase all the commits on top of the base branch, but we
don't want that. Having the merge commit is a nice indicator in the
history of groups of commits that were part of a PR. By having the
`strict: smart+fastpath` setting we are already guaranteeing that the PR
has been updated against the base branch.

See:
https://doc.mergify.io/actions.html#merge
and
https://doc.mergify.io/merge-action.html#strict-merge
@mmlb mmlb requested a review from grahamc June 4, 2020 17:40
grahamc
grahamc previously approved these changes Jun 4, 2020
@grahamc grahamc added the ready-to-merge Signal to Mergify to merge the PR. label Jun 4, 2020
mmlb added 2 commits June 4, 2020 13:51
According to the [docs][1] we can do away with a status check since
github already blocks the merge when we have branch protection in place.

[1]: https://doc.mergify.io/examples.html#automatic-merge-when-ci-works-and-approving-reviews
This way we ensure one set of reviews doesn't override a different set
of reviews by someone else. All reviewers should have their concerns
addressed before merging.
@grahamc grahamc merged commit 031f890 into tinkerbell:master Jun 4, 2020
@grahamc
Copy link
Contributor

grahamc commented Jun 4, 2020

It required a manual merge:

This pull request must be merged manually because it modifies Mergify configuration

@mmlb mmlb deleted the tweak-mergify-config branch September 30, 2020 21:42
@mmlb mmlb removed the ready-to-merge Signal to Mergify to merge the PR. label Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants