From e316251e7edfdbe34d45cc53aba972c497f47a33 Mon Sep 17 00:00:00 2001 From: adamnfish Date: Thu, 14 Mar 2024 12:31:47 +0000 Subject: [PATCH] Update wording to reflect teams' shared responsibility In a no-blame culture, all the work we produce is a shared responsibility. This means if a change causes an incident, it isn't the fault of the engineer that made the change. This cuts both ways though, if delivering software is a shared responsibility then this means reviewers aren't completely absolved. I believe this change better reflects the reality of how teams work here today. With the rise of AI-assistants, we will need to instill a renewed focus on code review and I believe that even more than now, a shared responsibility model for teams will be essential. This change serves as a trigger for checking that my sense of how this works is shared, ahead of rolling out guidelines for the use of AI assisted programming. --- pull-requests.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pull-requests.md b/pull-requests.md index 096e4b6..79e32ab 100644 --- a/pull-requests.md +++ b/pull-requests.md @@ -62,12 +62,12 @@ Good PR reviews are extremely valuable. They help share understanding of both th The following are generally applicable to PR reviews, but are not hard rules. -## A reviewer isn't responsible +## A reviewer isn't directly responsible The primary aim of a review is to confirm the PR addresses its stated problem in a reasonable way, and the solution has no obvious problems. "Reasonable" covers various aspects. For example: long-term maintainability by the team, monetary cost, security, etc. -The reviewer does not take responsibility for whether the code is correct, or will have unintended side-effects. +The reviewer does not directly take responsibility for whether the code is correct, or will have unintended side-effects. However, all the code produced and released by a team is a shared responsibility. When reviewing code, be sure to consider the human context. If this is a new contributor or a less experienced engineer, the teams' shared responsibility might mean that an experienced team member should give the review more time and attention. ## DRY No-one loves someone who writes the same comment fifteen times on a PR. If there is a recurring issue like naming, code formatting or code structure in a PR then point one example out, and make a general comment explaining there are multiple occurrences.