search

cfengine / cf-bottom

__init__
MIT License
3 stars 12 forks source link

Approve PRs that meet minimum requirements from repository #52

Closed nickanderson closed 4 years ago

nickanderson commented 4 years ago

It would be nice if our various automated tests could function as an approval.

For example, in CFEngine core we run Travis, and we can request Jenkins builds (with options for the difficulty of the jenkins gauntlent like exotics and deployment tests). It would be nice if tom would approve a PR (allowing merge) if both travis and Jenkins were green.

olehermanse commented 4 years ago

GitHub has required status checks, I think maybe it's better to use that system?

vpodzime commented 4 years ago

Yeah, traffic lights differ from approvals.

nickanderson commented 4 years ago

? my thought was more about being able to jump the human review queue for changes that pass some relatively high bar dunno how status checks work in combination with required reviews. required status checks would just raise our bar further, which is also probably ok, just not what I was after.

nickanderson commented 4 years ago

I had this thoguht when I introduced a new example in core over the weekend and was facing having to wait days for a human reviewer. @Lex-2008 saved me from pergatory though.

olehermanse commented 4 years ago

@nickanderson The reason for the required approval is that we want code to be reviewed (by humans). Making tom bypass that would defeat the purpose. (Also, you are an admin and can disable the required review in settings, if you really need to).