From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web11.8055.1601541858628159702 for ; Thu, 01 Oct 2020 01:44:18 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WzV75p52; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1601541857; h=from:from: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; bh=CjdO9SvEX23gPzdRhR0rpEgGO1woSK/est5npKrdRB0=; b=WzV75p52N8K1aksAKKYm5dLB0RKBUZ0CFMdKgDWxQkXiznw0DBJ+FL13YeNKsC94UtREIr K0kjLpqOsPPsPUdtwnifMhERIWLUUQqBQ22oYvVed5oKYzwsyDk7fuZxq8anPlqRSK1BHi WejY9kDN6iCi7CCD+WcDZ7pKrljUTLE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-445-a30BjLD8MZu_pvmFyrml5g-1; Thu, 01 Oct 2020 04:44:16 -0400 X-MC-Unique: a30BjLD8MZu_pvmFyrml5g-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B86FF107B768; Thu, 1 Oct 2020 08:44:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-110.ams2.redhat.com [10.36.113.110]) by smtp.corp.redhat.com (Postfix) with ESMTP id C73C51001281; Thu, 1 Oct 2020 08:44:11 +0000 (UTC) Subject: =?UTF-8?B?UmU6IOWbnuWkjTogW2VkazItZGV2ZWxdIFRpYW5vY29yZSBjb21tdW5pdHkgcGFnZSBvbiB3aG8gd2UgYXJlIC0gcGxlYXNlIHJldmlldw==?= To: Leif Lindholm , gaoliming Cc: devel@edk2.groups.io, jiewen.yao@intel.com, "'Guptha, Soumya K'" , announce@edk2.groups.io, "'Kinney, Michael D'" , 'Andrew Fish' References: <16383D375E5994D7.27235@groups.io> <005f01d69476$81768bd0$8463a370$@byosoft.com.cn> <20200928120131.GA5623@vanye> <009a01d6970b$a8d60100$fa820300$@byosoft.com.cn> <20200930101325.GE5623@vanye> From: "Laszlo Ersek" Message-ID: <6aeab706-9191-af72-c16f-bae0924880d7@redhat.com> Date: Thu, 1 Oct 2020 10:44:10 +0200 MIME-Version: 1.0 In-Reply-To: <20200930101325.GE5623@vanye> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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". 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. 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). Thanks, Laszlo