search

roc-lang / roc

A fast, friendly, functional language.
https://roc-lang.org
Universal Permissive License v1.0
4.42k stars 310 forks source link

for every PR require code review from a trusted contributor, even if the PR is from a trusted contributor. #1472

Open Anton-4 opened 3 years ago

Anton-4 commented 3 years ago

The way to go seems to be to:

The CODEOWNERS file should be set up to have at least two trusted contributors as code owners per folder. That way every PR will be reviewed by a (different) trusted contributor.

Anton-4 commented 3 years ago
bhansconnect commented 2 years ago

I think we should probably prioritize this given how much Roc is growing. At a minimum, it will help new users know who to request a review from. At a maximum, it will defend from a set of malicious users who intentionally review and submit a bunch of changes that we have to revert.

rtfeldman commented 2 years ago

Hm, so according to the docs adding someone to CODEOWNERS means they'll get automatically requested as a review on every PR.

That seems like it would get annoying and isn't really what we want here...but I don't see a way to turn it off! 😅

bhansconnect commented 2 years ago

If their are multiple names for a path it requires a review from one of the owners not all. So I think this should work out fine?

bhansconnect commented 2 years ago

Oh, I see. They automatically add all reviewers but only require one review...hmm

bhansconnect commented 2 years ago

Maybe we just want restricted write access? In this doc, it looks like we can require a review by either an admin or someone with write access. Not sure if we can do explicit write access to the trunk branch while still allowing everyone to have write access to other branches.

rtfeldman commented 2 years ago

Yeah that's what I'd like to do once the repo is ready to be public and I can move it into the roc-lang organization!

Unfortunately it's $4/user/month to have a private organization, and we already have 250 people in here. 🤑

There's no per-user cost to having it in a private personal repo (hence why roc is still under rtfeldman/ for now) but everyone has to have write access. There's no granular permissions option outside an organization.

Anton-4 commented 2 years ago

Hm, so according to the docs adding someone to CODEOWNERS means they'll get automatically requested as a review on every PR.

It can be set for specific folders and files, I would not mind being requested to review everything touching /editor, Earthfile .github...

Anton-4 commented 2 years ago

It appears that working with code owners is a premium feature available under the team plan.