From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by mx.groups.io with SMTP id smtpd.web10.9264.1601547748326368435 for ; Thu, 01 Oct 2020 03:22:28 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=FYPvV4DG; spf=pass (domain: nuviainc.com, ip: 209.85.221.52, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f52.google.com with SMTP id j2so5022559wrx.7 for ; Thu, 01 Oct 2020 03:22:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=sakrR5G5CtJKRK0Ch1ynKmJQk7AsGcX2j2XQwV+7fK8=; b=FYPvV4DGrHQFpHryUE+qjGwqq1WDa+t0baUISPag2Soq8tbuanA4QA8+pSMrDHHIuT CVlq5f3PdtUQylqdzBCi0FqCghKx3D+RESYzwrp2/UiZkJErLoKJtZCVZGcyX0m/zdxV 2EGbpA9m6kmoB7cX5/qSW+BpnEskDnLVfA4upQF+g5XxO2t8X3saItrwsHLPftYL90Bw llPM4XOUoWBg9t/SRr3btqIFx964pfdbJ1KklAhzHGNuIry43z24KXLYqddMXF1bSIYB 4vUzmdD1xwtXOkEbKQpKtSIeXMjJkOgPxsRZTxV6knFLWIpo3wuHHO8eHrDCFqAUsXaz i8Mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=sakrR5G5CtJKRK0Ch1ynKmJQk7AsGcX2j2XQwV+7fK8=; b=JXhZeXu2tyiVhERrQ7XFAaTYf6hgKw5CsUpywin+nu1Y+xTHtEmiefELlDh5DUSwrF /ybFxripZStxZ+tSD9VlptD22dMa/wzKqvID8JRhes0DBJ3wErheQXZXM4OAj+uk/31f Hr9NsBtr6XFXkL0e5N8u4dZfBy+7fJ9BDHPednN611hdyuiISODnqL2UpDG1Vgp3VleS N7nUVb0H6CQO585AtobW/LMkhCa39PRrUqkYNlzckZWleZ9kz4B3I0MowDG8S+/gow+E uKtNgxTp9p+pbbTuzmj1UCYdYIOzko//RWi2J1Kkrbis0Te+/KyjkaS5TFBPDE6Csygz VS9A== X-Gm-Message-State: AOAM532bzdXDsITMcDIeIGugptlP5NwLoUjjrvsBbaNfo3OjV0vDD8qR lJxAhUp0f+cap9MWTp82V2gycQ== X-Google-Smtp-Source: ABdhPJwxTTfDDCkL8IUDI/JRM7/fygAqI3bc1C+qZWFLYpdkf/Fkdv3JXiN4Sioul9ek5LxECT9q2A== X-Received: by 2002:a5d:4a0c:: with SMTP id m12mr8145641wrq.83.1601547746710; Thu, 01 Oct 2020 03:22:26 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id q20sm7781164wmj.5.2020.10.01.03.22.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Oct 2020 03:22:26 -0700 (PDT) Date: Thu, 1 Oct 2020 11:22:18 +0100 From: "Leif Lindholm" To: Laszlo Ersek Cc: gaoliming , devel@edk2.groups.io, jiewen.yao@intel.com, "'Guptha, Soumya K'" , announce@edk2.groups.io, "'Kinney, Michael D'" , 'Andrew Fish' Subject: =?UTF-8?B?UmU6IOWbnuWkjTogW2VkazItZGV2ZWxdIFRpYW5vY29yZSBjb21tdW5pdHkgcGFnZSBvbiB3aG8gd2UgYXJlIC0gcGxlYXNlIHJldmlldw==?= Message-ID: <20201001102218.GF5623@vanye> References: <16383D375E5994D7.27235@groups.io> <005f01d69476$81768bd0$8463a370$@byosoft.com.cn> <20200928120131.GA5623@vanye> <009a01d6970b$a8d60100$fa820300$@byosoft.com.cn> <20200930101325.GE5623@vanye> <6aeab706-9191-af72-c16f-bae0924880d7@redhat.com> MIME-Version: 1.0 In-Reply-To: <6aeab706-9191-af72-c16f-bae0924880d7@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Oct 01, 2020 at 10:44:10 +0200, Laszlo Ersek wrote: > On 09/30/20 12:13, Leif Lindholm wrote: > > Agreed. > > > > Reviever or Maintainer can approve a patch. Any Maintainer can push a > > patch that has been approved. > > I disagree. > > Assume Ard and myself are away and Jordan fails to report back in a week > or so, but Rebecca or Peter have reviewed a patch on the list for > OvmfPkg/Bhyve. > > In that case, the patch should *NOT* be merged by (for example) you, > just because you have push rights. The community will have to wait until > Ard, Jordan, or myself return and provide an ACK. > > If the maintainers are *consistently* irresponsive, then new maintainers > need to be added, possibly with a larger community discussion. But if > it's just a week (especially if we discussed our absence in advance), > then maintainer absence is completely sufficient and justified for > holding back patches, even if designated reviewers are OK with those > patches. > > I've been *really* disliking that, for example, the chief MdeModulePkg > reviewers don't regularly ACK patches that have been reviewed by > designated reviewers. If those reviewers are considered authoritative > enough to fully approve patches -- and most of them they have push > access already, anyway --, then we should rework Maintainers.txt so that > Maintainer roles be handed out with a finer granularity. If you will: > promote those reviewers to Maintainers, on their respective turfs. > > > This can happen either: > > - when the designated Maintainer for that patch is > > unavailable/unresponsive > > - if the patch submitter is also a Maintainer of some other part of > > the repo. > > > > No one can approve their own patches. > > > > The act of adding a Reviewer means delegating the approval work to > > them. > > I don't see it like that; I think Maintainers should have the last word > on every patch going in. And yes, this *requires* maintainers to be > responsive. > > ... Hm. Perhaps this is a sign that we really have two concepts here, > we've just not been distinguishing them clearly enough. Maybe we need to > split the reviewer role in two: "Approving Reviewer" and "Assistant > Reviewer". I think you're right. This is why we seem to have two sets of opinions on this topic. > For example, on OvmfPkg, I would suggest marking all current Reviewers > as "Assistant Reviewers". On ArmVirtPkg, I'd likely propose you as an > Approving Reviewer (you have stood in for Ard and myself anyway, for > years now), and suggest Assistant Reviewer role for Julien. Right, that makes sense to me. If I was to start bikeshedding, I might suggest adding an A: tag for approving reviewer. Possibly stealing the description from the current R: tag, and adding the approving bit. And maybe nicking the QEMU R: description outright for R:. > On > MdeModulePkg and other core packages, I'd defer the re-classification to > Intel; we'd likely see a really large number of Approving Reviewers > (justifiedly, I think). Agreed. / Leif