Thanks, Sage. I'll check that script. It's interesting to know how this has been done so far.
Yuri, thanks a lot for the detailed description of steps. So let's rephrase those ones in a different way: what should block a PR from being merged?
- The approval of a component lead/team members?
- Then let's put a "reviewed-by=@ceph/component-lead" condition. We just started to enforce this in Dashboard by using Code-Owners.
- Integration/E2E tests passing?
- As part of the PR checklist, both PR author or reviewers SHOULD identify whether a PR needs to pass a QA run and add the "qa-needed" label accordingly. In that case let's make "qa-needed" label block PR merges (or rely on the exiting rule for "DNM" label). Or we may create the opposite label "qa-not-needed" and assume that by default all PRs need QA?
- We may use this Github Action to ensure the PR checklist is followed (it'll add an "Incomplete Tasks" label if some checkbox is not ticked).
- Issue/Bug logging/status updates? Unfortunately this is out of the scope of Mergify, but it could be 'easily' automated via Github Hooks/Actions or Jenkins. I feel that we are missing workflows in Github issues, and maybe labels are used to fill that gap?
In any case, IMHO we should avoid falling back to unspecific conditions/labels, like using a "DNM"/"Ready-to-be-merged" label, as doing that would end up being more or less the same as clicking the Merge button.
I'm not sure how the dashboard development process works, but Yuri's questions and the issues they provoke worry me about any kind of automated merge system in the broader Ceph codebase. It's not easy to identify when satisfactory testing has been completed for a given PR (since teuthology is not integrated, and a run can have failures that do or don't matter for a given PR), and reviewed-by statements or Github approvals can be provided either before or after that teuthology testing is done, and sometimes contingent on it passing. (ie, "LGTM assuming tests pass")
I really don't see a way we can programmatically interrogate the necessary "win" conditions right now without requiring some kind of explicit label that is semantically equivalent to pushing the merge button. Plus, our leads already have workflows built up to automate some portion of those steps when they're comfortable.
From looking at how Mergify sells itself and is used in ceph-ansible, it seems like it works best when either
1) the merge conditions are very loose (ie, 1-2 approvals), or
2) the necessary reviews and test suites are very well-integrated into the GitHub API, are the sole qualifier for merging, and can be counted on to be accurate.
Unfortunately, neither of those apply to the Ceph project as a whole. :(
-Greg