From patchwork Wed Mar 1 16:13:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ludovic_Court=C3=A8s?= X-Patchwork-Id: 47509 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 0740016CC0; Wed, 1 Mar 2023 16:14:24 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 870D316CC5 for ; Wed, 1 Mar 2023 16:14:21 +0000 (GMT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pXP5g-0007Ql-L0; Wed, 01 Mar 2023 11:14:04 -0500 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 1pXP5f-0007QY-6j; Wed, 01 Mar 2023 11:14:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pXP5e-0005Xr-UH; Wed, 01 Mar 2023 11:14:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pXP5e-00012I-Js; Wed, 01 Mar 2023 11:14:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#61894] [PATCH RFC] Team approval for patches Resent-From: Ludovic =?utf-8?q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-devel@gnu.org, guix-maintainers@gnu.org, guix-patches@gnu.org Resent-Date: Wed, 01 Mar 2023 16:14:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 61894 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 61894@debbugs.gnu.org Cc: guix-devel , guix-maintainers@gnu.org X-Debbugs-Original-To: guix-patches@gnu.org X-Debbugs-Original-Xcc: guix-devel , guix-maintainers@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.16776872233941 (code B ref -1); Wed, 01 Mar 2023 16:14:02 +0000 Received: (at submit) by debbugs.gnu.org; 1 Mar 2023 16:13:43 +0000 Received: from localhost ([127.0.0.1]:54743 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pXP5K-00011V-SF for submit@debbugs.gnu.org; Wed, 01 Mar 2023 11:13:43 -0500 Received: from lists.gnu.org ([209.51.188.17]:35820) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pXP5I-00011L-G0 for submit@debbugs.gnu.org; Wed, 01 Mar 2023 11:13:41 -0500 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 1pXP5A-0007KG-Ku for guix-patches@gnu.org; Wed, 01 Mar 2023 11:13:34 -0500 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 1pXP59-0005RW-B3 for guix-patches@gnu.org; Wed, 01 Mar 2023 11:13:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:Subject:To:From:in-reply-to: references; bh=BcVypeuNB7JKifBjRW15e0NZWsomFfrCZXl1DG8SR60=; b=F8K5ZmgrpcwPcc FnjhhJXwYpBc9/XOJABvsS35hvNHaBrQaO2XXaE5OJ4kAZDXes2j6vGsDdtSV8XN/jWOdzN6Llmf/ 2LdR/G+nYGh54neyEol/BCi2yqKsN6OA4Naay+gU+Ql3B+3ooZ+QQYf+k0Mdh48gDMQqsm5vWGy98 wJJNQnMGSjuquD2l8o6ytKaRBWuaxZnD77vryzuQbUq3fcSVS7hIxmb0wnFe8UNkUQ+Y/LxsWAkZS LN6FlYuAidanbcJCWPEzdBBDABSVU1mIxt0J/oJoV5gvnE1nvTUtStpowgnwp/EIkENERi+rZ1ZXV TNVqqr+BEtVffVEaMaww==; Received: from 91-160-117-201.subs.proxad.net ([91.160.117.201] helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pXP58-0002jv-Ps for guix-patches@gnu.org; Wed, 01 Mar 2023 11:13:31 -0500 From: Ludovic =?utf-8?q?Court=C3=A8s?= X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: Primidi 11 =?utf-8?q?Vent=C3=B4se?= an 231 de la =?utf-8?b?UsOpdm9sdXRpb24s?= jour du Narcisse X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Wed, 01 Mar 2023 17:13:28 +0100 Message-ID: <878rgga1qv.fsf@inria.fr> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 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+patchwork=mira.cbaines.net@gnu.org Sender: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org X-getmail-retrieved-from-mailbox: Patches Hello Guix! The project has been steadily gaining new contributors, which is great, and I think we need to adjust our processes accordingly. Currently teams are described mostly as pools of people who can mentor contributors in a particular area and who can review patches in that area. My proposal is to give teams formal approval power over changes to code in their area. This is sorta happening already, but informally: if a non-committer sends a patch, someone from the team eventually “approves” it by pushing it. Within a team, the situation is different: people usually discuss changes, and the submitter (also committer) eventually pushes them; sometimes, the submitter pushes changes without getting approval (or feedback) from others on the team. With the proposed policy, members of a team would also have to review and approve each other’s work. Formal approval means getting an explicit “LGTM” (or similar) from at least one other team member. This is similar to the review thresholds found on GitLab & co., where project admins can specify a minimum number of approvals required before a change is marked as ready. I think it avoids the unavoidable misunderstandings that can arise in a growing group and help pacify day-to-day collaboration. Below is a suggested change. What do people think? Ludo’. diff --git a/doc/contributing.texi b/doc/contributing.texi index c436bc4a31..d8ca6802c4 100644 --- a/doc/contributing.texi +++ b/doc/contributing.texi @@ -1486,7 +1486,7 @@ reply to incoming bugs and patches, which contains the bug number. @anchor{Notifying Teams} @cindex teams The @file{etc/teams.scm} script may be used to notify all those who -may be interested in your patch of its existence (@pxref{Teams}). +may be interested in your patch and may approve it (@pxref{Teams}). Use @command{etc/teams.scm list-teams} to display all the teams, decide which team(s) your patch relates to, and use @command{etc/teams.scm cc} to output various @command{git send-email} @@ -1557,6 +1557,9 @@ these changes are necessary. @subsection Teams @cindex teams +The project is structured as @dfn{teams}, which play two related roles: +mentoring people who contribute code in their area of expertise, and +reviewing and approving changes to that code. There are several teams mentoring different parts of the Guix source code. To list all those teams, you can run from a Guix checkout: @@ -1840,8 +1843,14 @@ Patches}). It also allows patches to be picked up and tested by the quality assurance tooling; the result of that testing eventually shows up on the dashboard at @indicateurl{https://qa.guix.gnu.org/issue/@var{ISSUE_NUMBER}}, where -@var{ISSUE_NUMBER} is the number assigned by the issue tracker. Leave -time for a review, without committing anything (@pxref{Submitting +@var{ISSUE_NUMBER} is the number assigned by the issue tracker. + +When your patch falls under the area of expertise of a team +(@pxref{Teams}), you need the explicit approval of at least one team +member before committing (another team member if you are on the team). + +In other cases, +leave time for a review, without committing anything (@pxref{Submitting Patches}). If you didn’t receive any reply after one week (two weeks for more significant changes), and if you're confident, it's OK to commit.