Require four eyes


#1

I have a requirement that an author must not be allowed to merge his own pull request without another persons approval. My current take on this is to implement a Github bot that sets a green commit status as soon as this is the case and then have bors.toml contain

status = ["my-general-check", "different-person-approval"]

Has anyone else done something similar?


#2

Don’t overengineer it. This is really a people problem, not an engineering problem. Code reviews are only good if the developers believe in them. If the developers believe in the policy, then they’ll do a good job regardless of the bot. If they don’t, then they’ll learn to appease the bot with the right keystrokes, while neglecting any meaningful review.


#3

I appreciate your input. I fully fully agree on a personal level, but there are some external stakeholders requiring four eyes on our stuff…


#4

Depending on how much you want, maybe Reviewable would help? You can actually write reviewer policies in JavaScript, and it reports its results as a GitHub Status so you can use it with bors. I think @mithrandi uses it that way?


#5

You got it!

Via https://github.com/bors-ng/bors-ng/pull/362 you can now specify
required_approvals in your bors.toml now.


#6

Wow! You were fast! Thanks! What’s funny is I just put together https://github.com/tink-ab/four-eyes to solve this issue and had missed to track this thread… Rats!

Since my approach is more composable and doesn’t expand the scope of bors-ng, is https://github.com/bors-ng/bors-ng/pull/362 still a feature you’d like to keep in bors-ng? An alternative would be to revert the change and rely on four-eyes instead.


#7

Most of the work to implement it already had to be done to fix PRs requiring reviews fail bors silently anyway. Wish I had known, you could’ve saved a lot of work.

Don’t feel too bad about not staying on top of bors-ng’s roadmap; it really doesn’t have one :blush: