Automating PR merges proposal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

At Ceph-Dashboard's meetings we have discussed a couple of times how to improve PR review & merge process. Sometimes Pull Requests get stalled with no clear reason: they have positive reviews, perhaps some comments/suggestions, but not clear rejections. And, after a certain moment, the longer they remain open, the harder they seem to get merged (or closed).

An idea taken from Ceph-Ansible team would be to 'code' the set of rules that a PR needs to meet in order to be merged and use some tool (e.g.: Mergify) to implement that. This would change the contribution pipeline from a push model (PRs not merged unless manual action taken) to pull model (rules met, PRs get merged unless manual action taken).

This PR brings Mergify to Dashboard, with the following set of rules:
  • PR base branch is 'master'
  • PR is labeled 'dashboard'
  • PR title starts with "mgr/dashboard: "
  • All Jenkins checks pass (except arm64)
  • 2 review approvals required with no changes-requested or unaddressed comments. All requested reviewers have issued a review (perhaps too strict).
    • As Dashboard's CODEOWNERS PR is already merged, that means that at least 1 one the approvals must come from a @ceph/dashboard team member.
  • 'DNM' label not present
One downside identified is that Mergify does not allow to specify a message for the merge commit (e.g.: to add "Reviewed-by:" metadata). I'm preparing a PR to Mergify repo, but the GitHub Reviews API does not easily provide with every reviewer's e-mail (nor the Users API does), so it needs to be extracted from the Events API (which is kind of tricky). And this use case seem to be specific to us, right?

So, would it be ok to have PR merge commit messages with "Reviewed-By: FirstName LastName <@github_login>" or even with no message at all?

BTW, apart from the "merge" action, Mergify provides other actions that could be useful to automate other daily manual steps:
  • Backport the PR to another branch
  • Add a comment to the PR
  • Add or remove labels (e.g: if the PR title matches "^mgr/dashboard: ", let's add a "dashboard" label).
Any feedback welcome!

Kind Regards,

Ernesto Puerta

He / Him / His

Senior Software Engineer, Ceph

Red Hat

_______________________________________________
Dev mailing list -- dev@xxxxxxx
To unsubscribe send an email to dev-leave@xxxxxxx

[Index of Archives]     [CEPH Users]     [Ceph Devel]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux