From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 6E68B7400A0 for ; Wed, 21 Feb 2024 23:33:09 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=PuuRWTOjOjd9RUiSEl6eYUly0PCARx/MSmStfOs5BJk=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1708558388; v=1; b=UK4gddXpvM7fXTpmGK+HTD+btb1orCjD01oew4hVct0gQjeZyQdW43z7f8cBxeeAWRJ77Bx/ oQaNhoVceit69DhEisBzATtdv0LRnRtxmxj6qSsUSACAPvtePAh8TwtIzFMA9en3MCkrFMjdCUj mjtchHGV2D8y+aa7HUnBhybQ= X-Received: by 127.0.0.2 with SMTP id raHlYY7687511xPvZY4TPfzf; Wed, 21 Feb 2024 15:33:08 -0800 X-Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.1212.1708558387368766532 for ; Wed, 21 Feb 2024 15:33:07 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id BE451616AB for ; Wed, 21 Feb 2024 23:33:06 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3A8D8C433B2 for ; Wed, 21 Feb 2024 23:33:06 +0000 (UTC) X-Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2d1094b549cso103814281fa.3 for ; Wed, 21 Feb 2024 15:33:06 -0800 (PST) X-Gm-Message-State: cBj5P1h0Sh3T3KO3xaElCc7Dx7686176AA= X-Google-Smtp-Source: AGHT+IE/EmCtAUfT1niXVuvcw9KdLTuPFBe2F0TKbZCcDOvabEKGcN9hASLFIHcP+dqFJiveTb85VQ7An8YoUtQRPHM= X-Received: by 2002:a2e:81c6:0:b0:2d2:45e0:f154 with SMTP id s6-20020a2e81c6000000b002d245e0f154mr4382254ljg.26.1708558384256; Wed, 21 Feb 2024 15:33:04 -0800 (PST) MIME-Version: 1.0 References: <20240218205951.497-1-michael.d.kinney@intel.com> <20240218205951.497-4-michael.d.kinney@intel.com> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 22 Feb 2024 00:32:50 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [Patch 3/4] BaseTools/Scripts/PatchCheck: Error if no Cc tags are present To: devel@edk2.groups.io, lersek@redhat.com Cc: michael.d.kinney@intel.com, Rebecca Cran , Liming Gao , Bob Feng , Yuwei Chen , Michael Kubacki Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=UK4gddXp; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Wed, 21 Feb 2024 at 23:16, Laszlo Ersek wrote: > > On 2/20/24 15:48, Ard Biesheuvel wrote: > > Hello Mike, > > > > I understand the desire to be pedantic about cc'ing the right > > maintainers, but I'm not convinced this is the way. > > > > - the presence of a cc: tag does not guarantee that the person was > > cc'ed - only git send-email will take CC:s in the message body into > > account by default (but this can also be disabled), but generally, the > > sender has to ensure the cc list is copied into the cc: field > > - the absence of a cc: tag does not imply that the person was not cc'ed, > > > > - in Linux, the cc: tag has slightly different semantics from the ones > > we appear to be assuming here: a cc tag in patch going into the > > repository is a statement by the maintainer that the person in > > question has been cc'ed, may have some 'jurisdiction' over the area, > > but hasn't bothered to respond. IOW, it is to record the fact that > > this person has been given the opportunity to respond. > > > > Then there is the matter of a maintainer that has reviewed the patch > > themselves. I usually remove the cc lines listing people that have > > reviewed/acked/tested the patch, as those tags already convey that the > > person is aware of it cc'ed or not. > > I've noticed this (on patches you merged), and -- not having similar > maintainer experience in Linux -- I was surprised. I more or less > deduced the intent, but it felt a bit foreign (or at least novel!) to edk2. > > To me, the greatest benefits of Cc's in commit messages are (as opposed > to command line specified Cc's): > > - fine-grained: each patch can target a specific set of reviewers / > maintainers, > > - long-lived: the CC list survives rebases / v2, v3 etc iterations! (Of > course, if a patch undergoes serious scope changes when revised, then > the Cc list will have to be updated manually. But that's quite rare.) > > > > > So perhaps it would be better to make this check part of the > > contributor workflow but not the GitHub PR/CI workflow? > > I agree that adding Cc's to the commit message body is not fool-proof > (like you explain), but like Mike, I have no better idea for preventing > contributors from posting patches without properly CC'ing > reviewers/maintainers (be it with whatever particular CC'ing method they > prefer). > > I tend to run PatchCheck locally (not solely relying on CI for that -- > PatchCheck is quick and has no intrusive dependencies, plus seeing CI > fail just because of PatchCheck is super irritating), so in my workflow, > this patch would fit well. Of course, with the same effort of > remembering to run PatchCheck locally, I also remember to add Cc's in > the first place... > > I admit that reviewer assignment is a significant shortcoming of the > email-based workflow. What we'd really need is a groups.io-level action > :) -- if the subject contains PATCH or Patch in brackets, but the body > lacks ^Cc or ^CC, *reject* the email. (Rejection gives the sender an > explanation.) Alas, rejection is currently only a manual action that's > available to moderators (and only on messages for senders that have not > been unmoderated yet). > > So, my take: not perfect, but much better than nothing. > Yeah, I can't argue with that. I agree that it is very annoying that patches don't get cc'ed to the right people. (It is even more annoying that many maintainers [including myself at times - mea culpa] don't bother to respond, but that is a different matter) So let's try this, and in case it does more harm than good, we can always back it out again (or make modifications to the logic) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115752): https://edk2.groups.io/g/devel/message/115752 Mute This Topic: https://groups.io/mt/104434584/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-