From b8c62e7bc23b39752a608781225b650f6beae696 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 5 Dec 2023 08:05:58 -0500 Subject: [PATCH] governance: updating review changes (#30995) Risk Level: n/a Testing: n/a Docs Changes: yes Release Notes: no Signed-off-by: Alyssa Wilk --- CONTRIBUTING.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6ff4a8d508cc..a1f77ded5e73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -237,18 +237,19 @@ Please note that if adding a runtime guarded feature, your [release notes](chang * Typically we try to turn around reviews within one business day. * See [OWNERS.md](OWNERS.md) for the current list of maintainers. * It is generally expected that a senior maintainer should review every PR to - core code. Test-only or extension-only changes need only be reviewed by a - maintainer, or senior extension maintainer. + core code. Changes which only touch tests, extensions, tools, docs or comments + need only be reviewed by a maintainer, or senior extension maintainer. * It is also generally expected that a "domain expert" for the code the PR touches should review the PR. This person does not necessarily need to have commit access. -* The previous two points generally mean that every PR should have two approvals. (Exceptions can - be made by the senior maintainers). -* The above rules may be waived for PRs which only update docs or comments, or trivial changes to - tests and tools (where trivial is decided by the maintainer in question). -* In general, we should also attempt to make sure that at least one of the approvals is *from an +* For new extensions (contrib or otherwise) and features, at least one of the approvals should be *from an organization different from the PR author.* E.g., if Lyft authors a PR, at least one approver should be from an organization other than Lyft. This helps us make sure that we aren't putting organization specific shortcuts into the code. + new HTTP/3 features are largely exempt from cross-company approvals as all of the + area experts work at a single company, but HTTP/3 changes which impact general + functionality still merit a cross-company check. +* contrib extensions do not need senior maintainer or maintainer review only contrib owner review and + a maintainer stamp to merge. * If there is a question on who should review a PR please discuss in Slack. * Anyone is welcome to review any PR that they want, whether they are a maintainer or not. * Please make sure that the PR title, commit message, and description are updated if the PR changes