Begin using CODEOWNERS in this repo

[gene1wood]

https://help.github.com/en/articles/about-code-owners

Given that we have different groups of people working on different parts of the codebase (e.g. @andrewkrug and I working on CI/CD), if we define a CODEOWNERS file and then enable merging to follow that file, I can for example merge a change to CI/CD that doesn't affect the MozDef codebase without requiring @pwnbus to review and merge.

I wanted to see if this sounded ok before PRing a file for CODEOWNERS.

This would potentially have a section like

# These owners will be the default owners for everything in
# the repo. Unless a later match takes precedence,
# These users will be requested for
# review when someone opens a pull request.
*       @jeffbryner @pwnbus @mpurzynski @Phrozyn @tristanweir

# Require review by gene or andrew for cloudy MozDef stuff
/cloudy_mozdef/ @gene1wood @andrewkrug

Then we'd uncheck Restrict who can push to matching branches
And add a check to Require review from Code Owners

This way

• nothing could be merged without review
• the people required for review for everything other than cloudy mozdef would be the same list of people who can merge today
• the people required for review of cloudy mozdef would be andrew and I

Thoughts @jeffbryner @pwnbus @mpurzynski @Phrozyn @tristanweir?

[andrewkrug]

It would also be nice if everyone could update the README and Documentation.

[gene1wood]

So maybe something like

# These owners will be the default owners for everything in
# the repo. Unless a later match takes precedence,
# These users will be requested for
# review when someone opens a pull request.
*       @jeffbryner @pwnbus @mpurzynski @Phrozyn @tristanweir

# Require review by gene or andrew for cloudy MozDef stuff
/cloudy_mozdef/ @gene1wood @andrewkrug

# Anyone in EIS can modify documentation
README.md    @mozilla/enterprise-information-security
/docs/    @mozilla/enterprise-information-security

[tristanweir]

I think this is a great approach. I agree that /docs and README are fine to be permissive.

I assume
* @jeffbryner @pwnbus @mpurzynski @Phrozyn @tristanweir
means those users have the ability to review anything? I would think this would be a good failsafe for the cloudy mozdef stuff because if @gene1wood PRs something and @andrewkrug is on PTO, Gene might want someone else to review for a quick merge.

More on CODEOWNERS https://help.github.com/en/articles/about-code-owners (I was unfamiliar with the feature)

[gene1wood]

I assume * @jeffbryner @pwnbus @mpurzynski @Phrozyn @tristanweir
means those users have the ability to review anything?

Yes

[pwnbus]

I think this makes sense to do.

[tristanweir]

I wanted to see if this sounded ok before PRing a file for CODEOWNERS.

@gene1wood PR away!

[Phrozyn]

Sounds good!

[gene1wood]

Once #1237 is merged we'll want to change the branch protection settings

[gene1wood]

Then we'd uncheck Restrict who can push to matching branches
And add a check to Require review from Code Owners

[gene1wood]

Looks like we've got the Require review from Code Owners checked but we still need to uncheck the Restrict who can push to matching branches

[pwnbus]

I unchecked the "Restrict who can push to matching branches" so we should be all set?

[pwnbus]

Tristan had added you and Andrew to the list of folks that can merge code, so based on our discussion, I think we want to have "Restrict who can push to matching branches" in addition to having "Require review from Code Owners" checked.

[gene1wood]

We'll need to keep in mind going forward then that the controls for who can modify various files in the MozDef codebase is controlled by the intersection of the reviewers defined in CODEOWNERS and the users who have merge permissions.

If we want to modify CODEOWNERS to grant someone else rights to modify some aspect of the code (by approving a PR) we'll need to also add them to the list of users with rights to merge.