web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
4.99k stars 3.1k forks source link

Add CODEOWNERS for /common, /images, /resources, /tools, /lint.whitelist etc. #11256

Open gsnedders opened 6 years ago

gsnedders commented 6 years ago

I wonder if we should require some sort of super-review for top-level shared directories, tooling, and the lint.whitelist? Changing these has a far higher risk of breaking large numbers of tests.

cc/ @web-platform-tests/admins (we should probably also have some better group to ping people for meta issues)

jdm commented 6 years ago

Are the people who review changes to these different from the people who would be super-reviewing changes?

gsnedders commented 6 years ago

@jdm In the typical case, no. I expect myself and @jgraham would own all of /tools, for example, and the majority of commits are already reviewed by one of us. The point is mostly just requiring someone in a strict subset of reviewers to sign off on changes in them.

jgraham commented 6 years ago

FWIW I would be inclined to add such rules for tools/, resources/ and *.py, since the first two have real ownership requirements (and active owners) and the last is most of the security surface we expose. I'm less bothered about common/ and images/ and in particular don't want to discourage people from adding things there due to stricter review requirements.

All of that said, I don't know how this would interact with repo sync; I think in reality upstream changes for these things would need to be permitted.

gsnedders commented 6 years ago

Yeah, it might not be worth adding to common/ and images/. I guess really what I want is something that is roughly "these directories are append only". I'd definitely want it on lint.whitelist, though.

foolip commented 6 years ago

Requiring extra review on lint.whitelist would be a problem, log -p --grep Cr-Commit-Position -- lint.whitelist will show you a bunch of commits there, and at a glance the changes are good ones.