From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:403:478a::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms9.migadu.com with LMTPS id 6EO4JTBSJWWYzgAAauVa8A:P1 (envelope-from ) for ; Tue, 10 Oct 2023 15:31:28 +0200 Received: from aspmx1.migadu.com ([2001:41d0:403:478a::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id 6EO4JTBSJWWYzgAAauVa8A (envelope-from ) for ; Tue, 10 Oct 2023 15:31:28 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 167E2D9CA for ; Tue, 10 Oct 2023 15:31:28 +0200 (CEST) Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=GUnCW2fo; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1696944688; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=A3/wQvjXHC0vcxWatXpUTvqO71aCQbOxALV95NNyAmo=; b=fD2TM3RnEI0JKJ0HIUtwQdzpu3t8lFWrhmGPrSl6j9ok7Ne/fnwvJ7t15Sk9AFHZkvZk8G Bc0u65BWMhTgkGH26uaYkYl7qxkIYCNGkAjZV0SVENg2FFP7cIhTZ+ZcrekNxVnywFXANF ZMsxXi0VqHvxWL5OT7UGzZFnK4j+SGeh7D3ojXPYL5mC5gM9vKiqqjgA8tcJqkuN1eLrpw 8AmIxsZFKQUHtJQP3vS1nBmQsmkQNX4jcVkHnX2ljBHbvIzI9jbjnVhnEh6tsEXoncCkju fGduJR+PmdHMFct0yz3jqmwb4RZjynSNe77vbjQU9dH6EKmQ39VdDSDD4yh2OA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1696944688; a=rsa-sha256; cv=none; b=HHmnuQKTIyTdA55JIDk9xXf0ggGKX4izngSFcLJp3K9IvF+s/6SD4CF4UIAFvqwtZ+Vdzh cmLxhQvpRDeoK0JEz5+HMKTpHg+mR7gR0i54eMv6FRHAAXiDa/pAkktom0SEcUe5H8/TLl W0QUTIcH5nibfpjlFqxndlHMz4DhdwSElMbAvLqZKosxdWNL3SNinPOamJPGkNZcWoFAXb +f404Xsb2my9FV5pRwzqtGHiIHwOgohtf4i/RbQuHEafv8hRfAbm8wA8Szu7ZnRmGmXGYo mPlVcYCbYoRZHFedTxvRqn4ERkI14tdPQ1oQpJSGufikS0w04uBuut9nF0qEig== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=GUnCW2fo; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqCp2-0006oD-RE; Tue, 10 Oct 2023 09:30:52 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqCot-0006mj-SG for guix-patches@gnu.org; Tue, 10 Oct 2023 09:30:43 -0400 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqCor-0004Z8-QC for guix-patches@gnu.org; Tue, 10 Oct 2023 09:30:43 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qqCpC-00042o-VX for guix-patches@gnu.org; Tue, 10 Oct 2023 09:31:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#66436] [PATCH] doc: Add some guidelines for reviewing. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 10 Oct 2023 13:31:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 66436 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxim Cournoyer Cc: 66436@debbugs.gnu.org Received: via spool by 66436-submit@debbugs.gnu.org id=B66436.169694462415468 (code B ref 66436); Tue, 10 Oct 2023 13:31:02 +0000 Received: (at 66436) by debbugs.gnu.org; 10 Oct 2023 13:30:24 +0000 Received: from localhost ([127.0.0.1]:34322 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qqCoZ-00041N-8i for submit@debbugs.gnu.org; Tue, 10 Oct 2023 09:30:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58368) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qqCoW-000419-Md for 66436@debbugs.gnu.org; Tue, 10 Oct 2023 09:30:21 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqCo6-0004C8-3R; Tue, 10 Oct 2023 09:29:54 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:References:In-Reply-To:Subject:To: From; bh=A3/wQvjXHC0vcxWatXpUTvqO71aCQbOxALV95NNyAmo=; b=GUnCW2foClk8GJJEmvq/ tk9YXoDYJUHKAhwpjRF9yrfgkyZG/KhrWAekIoCqLr3XdJ2g6bjExf3NAe0QaX/n4WQ7w2B/s1Fjd 72SJWxujnpWMR3hv4EPXFoW+Srmrm8csZav8dPXw+UmrnZfhRLViOxbqvqh1nw7sMyqi4dAroNOFr kG0CQ/K1+l7TEHbCVxH07N7Cik42ogdhJ2yzwauJUlmBAuOl51cwloKUdi0+FVsp+QI00qrD6sbGu PLfL0EuyAybWLyTHCYNfUXMv/+bUpE9tEHRL+JTjfzKT5FPXsapjXneHGodzpRo8P8n1xPFlSDM8j Fgqxycah3LHgHw==; From: Ludovic =?UTF-8?Q?Court=C3=A8s?= In-Reply-To: <19f82d9bbef649c750ad067d23ebbaee6f9ae494.1696942467.git.maxim.cournoyer@gmail.com> (Maxim Cournoyer's message of "Tue, 10 Oct 2023 08:54:27 -0400") References: <19f82d9bbef649c750ad067d23ebbaee6f9ae494.1696942467.git.maxim.cournoyer@gmail.com> Date: Tue, 10 Oct 2023 15:29:37 +0200 Message-ID: <87r0m2wqpq.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US X-Migadu-Scanner: mx0.migadu.com X-Migadu-Spam-Score: -6.91 X-Spam-Score: -6.91 X-Migadu-Queue-Id: 167E2D9CA X-TUID: ozoUBYV22UR3 Hi, This looks great to me! Maxim Cournoyer skribis: > +@cindex LGTM, Looks Good To Me > +@cindex reviewing, guidelines > +Review comments should be unambiguous; be as clear and explicit as you > +can about what you think should be changed, ensuring the author can take > +action on it. I=E2=80=99d add a few guidelines here, and perhaps we can make it a bullet list: 1. Be clear and explicit about changes you are suggesting, ensuring the author can take action on it. In particular, it is a good idea to explicitly ask for new revisions when you want it. 2. Remain focused: do not change the scope of the work being reviewed. For example, if the contribution touches a code that follows a pattern deemed unwieldy, it would be unfair to ask the submitter to fix all occurrences of that pattern in the code; to put it simply, if an problem unrelated to the patch at hand was already there, do not ask the submitter to fix it. 3. Ensure progress. As they respond to review, submitters may submit new revisions of their changes; avoid requesting changes that you did not request in the previous round of comments. Overall, the submitter should get a clear sense of progress; the number of items open for discussion should clearly decrease over time. 4. Review is a discussion. The submitter's and reviewer's views on how to achieve a particular change may not always be aligned. As a reviewer, try hard to explain the rationale for suggestions you make, and to understand and take into account the submitter's motivation for doing things in a certain way. 5. Aim for finalization. Reviewing code is time-consuming. Your goal as a reviewer is to put the process on a clear path towards integration, possibly with agreed-upon changes, or rejection, with a clear and mutually-understood reasoning. Avoid leaving the review process in a lingering state with no clear way out. I just made it up but I=E2=80=99m sure there are good review guidelines out there that we could use as an example. My 2=C2=A2, Ludo=E2=80=99.