From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:bcc0::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id 34zKMwTwgWB8KgAAgWs5BA (envelope-from ) for ; Thu, 22 Apr 2021 23:52:04 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id iKlKLgTwgWDLOQAAB5/wlQ (envelope-from ) for ; Thu, 22 Apr 2021 21:52:04 +0000 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 7F4FB1B70C for ; Thu, 22 Apr 2021 23:52:04 +0200 (CEST) Received: from localhost ([::1]:50838 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lZhEx-0005Hu-Mt for larch@yhetil.org; Thu, 22 Apr 2021 17:52:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:55974) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lZhEZ-0005GF-2P for guix-devel@gnu.org; Thu, 22 Apr 2021 17:51:39 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:57787) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lZhEW-0008PV-Ia; Thu, 22 Apr 2021 17:51:36 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=38398 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lZhEV-0007E5-Hs; Thu, 22 Apr 2021 17:51:36 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Guix Devel Subject: Re: Another misleading commit log (was Re: A "cosmetic changes" commit that removes security fixes) References: <87tunz11mf.fsf@netris.org> <87y2daz13x.fsf@netris.org> <87r1j2z079.fsf@netris.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 3 =?utf-8?Q?Flor=C3=A9al?= an 229 de la =?utf-8?Q?R?= =?utf-8?Q?=C3=A9volution?= 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: Thu, 22 Apr 2021 23:51:33 +0200 In-Reply-To: (=?utf-8?Q?=22L=C3=A9o?= Le Bouter"'s message of "Thu, 22 Apr 2021 22:06:41 +0200") Message-ID: <87mttqugwq.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Raghav Gururajan , Leo Prikler Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1619128324; 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: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=2kSJT0kdVamYYYuTiiSjXZnjTDj47a8R8IKrd6RuclE=; b=iSN/uXjUQDboPIQSes2J+d/IyyGzKOLIk+x06djapYWI+v+RIWf1WTcvtrAA0JO65CHjRp v5n3M55KixQjeDuSs8bfq+DrEOdAOMabCuoLI/fBG1xmLyTwXyLj4kBcXQwlD/QVleqsdK qb5onZuqIyRsMBxeAyOeOUzBAxHxvCvOj+hWrMui6eWNm051B3VP2t/3tVrhHxKy1PPded neSifX9adpfeqW9I/HGYgNCXyZK+aPTsC0P951MX+HGPUw2ie37nZn2QSyVPAszLGUZFvS /erd/1ondfcDTH8sf9n1MNDlXJhu41iROeW2E6Aq9uYY6S72KHyisAeedQnIHA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1619128324; a=rsa-sha256; cv=none; b=odYHe/qxtrAmDjsPBx7XUt+cT1Ve3fVG2QDdT7Mh/dyZ0MdN8fuIfyf2qkFQfZr77qyUCa v3jDcV5evYhRkQqMWx065vEyg69WOnXLltf6AtPLSwRV5RPviZ17jpkpX181r0Nl4BkrnN GgK/KJNbEwO1KvPR0uIcCXI6VmxRvvS/rYodHaOWyRMf5vR0LzI38F2EfYlIC8KRT+UU5U tfnzXcuiNCjKwrqxBNF2kAcbIQCx1p4ay282qDaZieBNQYRmyGwlmHYXkU1egdu860XOVs +G3r+NHZwI3Nwi01llmNxdzSe4stqhOqWbA4UZ+KM2j+pK1VYP9PTW2L11O1FQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Spam-Score: -2.95 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: 7F4FB1B70C X-Spam-Score: -2.95 X-Migadu-Scanner: scn0.migadu.com X-TUID: A8UJZyQUEDkI Hi Guix! Thanks Mark for raising these issues. I definitely share your concerns, specifically regarding the two commits you mentioned and how they misleadingly have undesirable consequences: https://git.savannah.gnu.org/cgit/guix.git/commit/?h=3Dwip-gnome&id=3Dd97= 5ed975456a2c8e855eb024b5487c4c460684a https://git.savannah.gnu.org/cgit/guix.git/commit/?h=3Dwip-gnome&id=3Df5f= c3c609e2f38ca1c0523deadb9f77d838fbf32 I believe all the parties involved behaved in good faith and I=E2=80=99m mo= re interested in (1) fixing these two mistakes, and (2) making sure this doesn=E2=80=99t happen again in the future. Regarding #1, I see =E5=AE=8B=E6=96=87=E6=AD=A6 (iyzsong) proposed a patch = that reinstates the Cairo security fixes, so it seems we=E2=80=99re on the right track. Raghav, could you post a patch reinstating the gdk-pixbuf security fix removed in f5fc3c609e2f38ca1c0523deadb9f77d838fbf32? If that commit is only on =E2=80=98wip-gnome=E2=80=99, can you simply drop it? (The conventi= on is that a =E2=80=98wip-=E2=80=99 branch may be rebased at will by the person responsi= ble for it.) Could you also check =E2=80=98wip-gnome=E2=80=99 and =E2=80=98core-updates= =E2=80=99 for similar =E2=80=9Ccosmetic changes=E2=80=9D commits likely to be controversial? Regarding #2, everyone please keep in mind the commit rules=C2=B9. We=E2= =80=99re all pouring hours of our time into this. It=E2=80=99s a social project before = being a technical one, so it=E2=80=99s crucial to not step on each other=E2=80=99= s toes. As per these rules, =E2=80=98wip-gnome=E2=80=99 will have to go through rev= iew as usual. As always, it=E2=80=99s best if you can submit it for review in small chunk= s. Note that review has to happen via guix-patches@gnu.org so everyone in the project can see it and has a chance to chime in. You can have pre-review/mentoring elsewhere if you want (it=E2=80=99s great if you can h= ave that), but it =E2=80=9Cdoesn=E2=80=99t count=E2=80=9D from the project=E2= =80=99s viewpoint. I think committers and particularly vouchers should spend more time reviewing and mentoring newcomers. Regarding commit logs, we only add a =E2=80=98Signed-off-by=E2=80=99 line w= hen applying someone else=E2=80=99s patches; let=E2=80=99s keep following that rule, for= clarity. For the subject line: as =E5=AE=8B=E6=96=87=E6=AD=A6 wrote, as a rule of th= umb, if you cannot summarize the change in the subject line, that probably means the patch should be split into smaller logical units. More generally, never bundle together unrelated changes in the same commit, as per: https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html Here are additional rules I try to follow for the commit message and that I=E2=80=99d recommend following: 1. Never write =E2=80=9Ccosmetic changes=E2=80=9D as the summary: it=E2= =80=99s too vague. Instead, be more specific: =E2=80=9Creindent foo.scm=E2=80=9D, =E2=80= =9Cremove unused module imports=E2=80=9D, whatever. 2. Never write =E2=80=9CFix X=E2=80=9D. Instead describe the fix: =E2=80= =9CAvoid non-top-level 'use-modules'=E2=80=9D, =E2=80=9CRemove file that no lon= ger exists=E2=80=9D, =E2=80=9CHonor proxy settings=E2=80=9D, =E2=80=9CDisable tests when bu= ilding on i586-gnu=E2=80=9D, etc. In all these examples I could have written =E2=80=9CFix =E2=80= =A6=E2=80=9D, but fellow hackers would have had to look at the diff to understand what=E2=80=99s going on. A long message! Let=E2=80=99s calm down and focus the way forward: fixing the security issu= es that were reported, checking whether similar ones are lurking, and improving our practices. If you feel the need for it, feel free to propose improvements to the =E2=80=9CCommit Access=E2=80=9D and =E2=80=9CSu= bmitting Patches=E2=80=9D sections, too! Thanks in advance. :-) Ludo=E2=80=99. =C2=B9 https://guix.gnu.org/manual/en/html_node/Commit-Access.html