From ca58c0d472655f1bb8ba64d0a03112908756e24f Mon Sep 17 00:00:00 2001 From: gnuxie Date: Fri, 21 Nov 2025 23:55:53 +0000 Subject: [PATCH 1/7] Begin documenting a basic review process --- design/2511-contribution-risk-and-response.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 design/2511-contribution-risk-and-response.md diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md new file mode 100644 index 0000000..643c5a7 --- /dev/null +++ b/design/2511-contribution-risk-and-response.md @@ -0,0 +1,59 @@ +# Contribution risk and response + +## Quick contribution risk check + +Does any of these contexts apply to the contribution? + +1. [ ] Security + +2. [ ] Infrastructure + +3. [ ] Uncertainty + +4. [ ] Impact of failure and or bugs + +If you are going to do any critical thinking and review, please think critically +about how the contribution impacts the system first. These are a guideline but +you must use your own judgement to determine the level of risk. + +## Risk classification + +- minimal if none apply + +- sensitive if one apply + +- critical if two or more apply. + +## Contexts + +### Security + +The contribution has subject matter related to security. Examples include: + +- Authentication +- Cryptography (including policy hashes) +- Untrusted input +- Access control (both host and guest (ie Matrix)) + +### Infrastructure + +The contribution has subject matter related to infrastructure. Examples include: + +- Changes in dependencies +- Changes to CI +- Changes to contributor workflow +- Changes to the repository + +### Uncertainty + +The contribution carries a lot of uncertainty. Examples include: + +- Reviewer lacks subject knowledge and must trust other experts. +- Large Contribution (300-500+). +- Lack of upfront planning or design. +- Limited communication with contributor. + +### Impact + +Consider specifically how problems in the change would be triaged if they were +buggy. From c1b8f84cce0872717dbd66479e90245f6b4a988f Mon Sep 17 00:00:00 2001 From: gnuxie Date: Sat, 22 Nov 2025 00:19:15 +0000 Subject: [PATCH 2/7] Note about the purpose of the document. --- design/2511-contribution-risk-and-response.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md index 643c5a7..1602f4d 100644 --- a/design/2511-contribution-risk-and-response.md +++ b/design/2511-contribution-risk-and-response.md @@ -1,5 +1,12 @@ # Contribution risk and response +This document is about assessing and responding to risk. This document does not +change the nature of review. Reviews are supposed to be optimistic and +encouraging, using risk as an excuse to block the author of a contribution will +be considered a failure to follow the process. The obligation to help +contributors achieve their goals and merge the PR lies with the maintainers, not +the contributor. Always be prepared to make requested changes yourself. + ## Quick contribution risk check Does any of these contexts apply to the contribution? From b76e150992c921ce42d65e39e35e2c78681909ab Mon Sep 17 00:00:00 2001 From: gnuxie Date: Mon, 24 Nov 2025 10:28:52 +0000 Subject: [PATCH 3/7] Create a Contribution risk and response document. --- design/2511-contribution-risk-and-response.md | 63 ++++++++++++++++--- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md index 1602f4d..de4bd14 100644 --- a/design/2511-contribution-risk-and-response.md +++ b/design/2511-contribution-risk-and-response.md @@ -2,10 +2,11 @@ This document is about assessing and responding to risk. This document does not change the nature of review. Reviews are supposed to be optimistic and -encouraging, using risk as an excuse to block the author of a contribution will -be considered a failure to follow the process. The obligation to help -contributors achieve their goals and merge the PR lies with the maintainers, not -the contributor. Always be prepared to make requested changes yourself. +encouraging. The obligation to help contributors achieve their goals and merge +the PR lies with the maintainers, not the contributor. Always be prepared to +make requested changes yourself to see the contribution through. However +reviewers are never obliged to approve a change if they are not confident in +change's the correctness or safety. ## Quick contribution risk check @@ -25,11 +26,11 @@ you must use your own judgement to determine the level of risk. ## Risk classification -- minimal if none apply +- Minimal if none apply -- sensitive if one apply +- Sensitive if one apply -- critical if two or more apply. +- Critical if two or more apply. ## Contexts @@ -64,3 +65,51 @@ The contribution carries a lot of uncertainty. Examples include: Consider specifically how problems in the change would be triaged if they were buggy. + +## Risk Response + +### Quick checklist + +- Sensitive changes and above should have a test plan. +- Sensitive changes and above should have the pull request checked out by the + reviewer. +- Sensitive changes and above should have all security sensitive code analysed + and discussed. + +### Checking out the PR + +Check out the PR locally, don't just rely on the webview. Go through the changes +files in your editor (use +https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github). + +Make sure everything is as expected. For changes involving UI, run the bot +locally and try to evaluate the feature yourself. Following the test plan if +available. + +### Reviewing security sensitive changes + +Think pessimistically about authentication and control flow through the +application. And think about how we arrive to the portion of code in question, +and where we go next. Think about the bigger picture and then the smaller +details. If anything is unclear, you MUST not continue and ask for clarification +or get someone else involved. + +### Test plans + +Pull requests with sensitive or critical risk should include a plan that +demonstrates the testing that the contributor has taken. This should be detailed +enough so that the reviewer can reproduce. + +## Expectations management + +Always try to get upfront communication with contributors to exchange +expectations before they commit to significant work. New contributors are often +ambitious, and so it is important to try get them to scale down their work or +even plan it to maximise their chances of success. + +This is especially important because big changes made by unfamiliar contributors +will always carry significant risk. Even when these changes concern +documentation. + +When an unsolicited pull request is opened, it's important to try establish +these expectations retroactively. And to identify and communicate risk. From 18e4959e182be9557a0b1a9cbf4aa1e6ca744583 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Mon, 24 Nov 2025 19:45:28 +0000 Subject: [PATCH 4/7] Pull requests labels. --- design/2511-contribution-risk-and-response.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md index de4bd14..a0a948d 100644 --- a/design/2511-contribution-risk-and-response.md +++ b/design/2511-contribution-risk-and-response.md @@ -24,6 +24,8 @@ If you are going to do any critical thinking and review, please think critically about how the contribution impacts the system first. These are a guideline but you must use your own judgement to determine the level of risk. +Use the pull request labels to assign risk contexts to pull requests. + ## Risk classification - Minimal if none apply From 2757f9bccb5ce0388ee144aa56a7f36f00914c86 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Mon, 24 Nov 2025 19:52:14 +0000 Subject: [PATCH 5/7] typo --- design/2511-contribution-risk-and-response.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md index a0a948d..5dfea44 100644 --- a/design/2511-contribution-risk-and-response.md +++ b/design/2511-contribution-risk-and-response.md @@ -5,8 +5,8 @@ change the nature of review. Reviews are supposed to be optimistic and encouraging. The obligation to help contributors achieve their goals and merge the PR lies with the maintainers, not the contributor. Always be prepared to make requested changes yourself to see the contribution through. However -reviewers are never obliged to approve a change if they are not confident in -change's the correctness or safety. +reviewers are never obliged to approve a change if they are not confident in the +change's correctness or safety. ## Quick contribution risk check From 77a3b96a1d7c09139ad43d4adf7e68f25e6551bf Mon Sep 17 00:00:00 2001 From: gnuxie Date: Wed, 26 Nov 2025 10:20:15 +0000 Subject: [PATCH 6/7] Add trust under sight section on reviewing summarised contributions. --- design/2511-contribution-risk-and-response.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md index 5dfea44..204e869 100644 --- a/design/2511-contribution-risk-and-response.md +++ b/design/2511-contribution-risk-and-response.md @@ -93,8 +93,8 @@ available. Think pessimistically about authentication and control flow through the application. And think about how we arrive to the portion of code in question, and where we go next. Think about the bigger picture and then the smaller -details. If anything is unclear, you MUST not continue and ask for clarification -or get someone else involved. +details. If anything is unclear, you MUST not continue, and instead ask for +clarification or get someone else involved. ### Test plans @@ -115,3 +115,12 @@ documentation. When an unsolicited pull request is opened, it's important to try establish these expectations retroactively. And to identify and communicate risk. + +## Trust under sight + +When a contributor summarises work, it's very important to recognise that this +summary is a declaration of intent, and can only be used within the context of +expectations management. A summary of changes in the pull request description, +can never be trusted to provide an exhaustive list of changes made in the +contribution. It is essential to check the substance of any contribution in +exhaustion, irregardless of any surrounding context. From c43ddae4ec2dcf7d76452fc8d0b03976e87b8a80 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Tue, 9 Dec 2025 14:23:28 +0000 Subject: [PATCH 7/7] Add a note about LLMs --- design/2511-contribution-risk-and-response.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/design/2511-contribution-risk-and-response.md b/design/2511-contribution-risk-and-response.md index 204e869..31d37cc 100644 --- a/design/2511-contribution-risk-and-response.md +++ b/design/2511-contribution-risk-and-response.md @@ -1,5 +1,12 @@ # Contribution risk and response +> [!IMPORTANT] +> +> We are unable to accept LLM assisted or generated contributions, as we do not +> feel confident that it is possible for any party to fulfill their obligations +> under NLnet's generative AI policy +> https://nlnet.nl/foundation/policies/generativeAI/. + This document is about assessing and responding to risk. This document does not change the nature of review. Reviews are supposed to be optimistic and encouraging. The obligation to help contributors achieve their goals and merge