I’ve enabled auto merge functionality on foreman-packaging. Do note that Github still marks this functionality as beta. I’d like to hear some feedback if people like it and if we should enable it for more repositories.
2 Likes
I like this, but if i’m not mistaken it currently only works for required checks, which we don’t have a lot of right now. For packaging i wonder if it makes sense right now, since some changes require manual follow-up (e.g. debian build) that may be forgotten if a reviewer auto-merges and forgets about it?
Hmm, good point. That is really painful since I assumed the build had to be green. I think we can set required checks per branch, but we often have to merge with a broken repoclosure due to our build pipelines. Given that, I think it may be better to disable auto merging again and first analyze the required checks properly.
As a test I enabled rpm as a required check on rpm/develop to see if we run into issues.
If you enable auto-merge for a pull request, the pull request will merge automatically when all required reviews are met and status checks have passed.
But what are these checks? Does this include at least one review from somebody? Or two?
I suspect it’s using branch protection rules. For those who don’t know how that settings page looks:
So there we could select some status checks that need to pass. In our repositories we have very few actually required status checks. One example we do have:
Note the Required label for Redmine issues.
So if we really want to utilize this feature, we should set up branch protection rules properly. Here’s the catch: sometimes our test suite fails for an unrelated reason. With required checks, it requires an admin to merge it with a failed check.
I’d be happy if our test suite was stable and we could require these checks, but I’m not sure if we’re there. It could be a good motivation to get there though.
In short: this is something we as maintainers need to agree on. I alone can’t make this choice.