OBOFoundry.github.io icon indicating copy to clipboard operation
OBOFoundry.github.io copied to clipboard

Consider auto-merge of approved PRs

Open cmungall opened this issue 4 years ago • 4 comments

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request

I can see various reasons to hold off on this, I think we would have to weigh the pros and cons of this.

I think this would be a good topic of discussion for a much needed TWG meeting, perhaps February or after

cmungall avatar Jan 11 '22 16:01 cmungall

Not sure how much bang you will get from this - its often harder to elicit > 1 review, most PRs can quickly be merged by the docs master.. But I see some advantage if we can agree on a three reviewer rule for PRs.. Not too pressing though atm.

matentzn avatar Jan 17 '22 13:01 matentzn

The idea of auto-merge is that you can specifically tell it to merge when all "checks" have passed. For example, in PyKEEN, we have a testing suite that takes 10 minutes. Sometimes we just want it to automatically merge when the suite ends, and not have to check back (any time we do this, it's a good time to go for a stretch). The checks can also be extended to have a certain number of approves as well, which means someone like you or Chris can approve something in principle, and have it automatically merge if there are other people who approve it too (or just wait until later and then manually merge it if the automerge condition is never met)

In practice this could reduce your and chris's efforts since you are responsible for review and merge of 90%+ of the PR review and merging anyway.

cthoyt avatar Jan 17 '22 13:01 cthoyt

You mean merge without human review? I would rather recruit some humans to do the mostly simple job of reviewing the PRs here than to relying on automerges.. There are so many bad things that people could do that would not fail any checks, like changing someone else's ontology description to "DO NOT USE UNLESS YOU ARE INSANE"..

matentzn avatar Jan 17 '22 15:01 matentzn

Reading the description of what auto-merge does, so far as I can tell, implementing this would simply make it so that a human would not have to click 'Merge PR' and the follow-up confirmation if all human reviews are 'Approve' and all checks come back as okay. If that's correct, it doesn't seem like it would hurt to implement. The main benefit is that the PR won't have to sit around and wait after all is deemed well. I know that I sometimes forget to go back and do the manual clicks.

nataled avatar Jun 10 '25 17:06 nataled