From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web10.5119.1588245906002992254 for ; Thu, 30 Apr 2020 04:25:06 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fKLvcxqG; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588245905; 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=mUSfYjC1RV5v3Ol9EpYFE9VrhdZdIxpriHKKx0ohuiY=; b=fKLvcxqGu23xi34MXawTRWkAbeQAi1D4ksdbNuocNOVo78fLTj3MRw14+3vRiSunBWCopN PKGAy3YCa+fzZ+aRzzGMl7E3JFq0KUIj42bFpAr0n7tm2l2KQS/1n3jtf0e8TtT+0MzvJG OA6LvFGOGvelNiK3ovAf7ByyGSACuD0= 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-231-mvfarSLPNe27TDvEIyQJzQ-1; Thu, 30 Apr 2020 07:25:03 -0400 X-MC-Unique: mvfarSLPNe27TDvEIyQJzQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 84A90835B42; Thu, 30 Apr 2020 11:25:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-57.ams2.redhat.com [10.36.115.57]) by smtp.corp.redhat.com (Postfix) with ESMTP id DABCF5C1BE; Thu, 30 Apr 2020 11:24:58 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 0/7] Edk2 Platform and Core CI for ArmVirtPkg, EmulatorPkg, and OvmfPkg To: "Gao, Liming" , Sean Brogan , "devel@edk2.groups.io" , "michael.kubacki@outlook.com" Cc: Andrew Fish , Ard Biesheuvel , Bret Barkelew , "Justen, Jordan L" , Leif Lindholm , "Kinney, Michael D" , "Ni, Ray" , Sean Brogan References: <38d73ec4-ce01-d6a6-c2dd-5e925afa2084@redhat.com> From: "Laszlo Ersek" Message-ID: <16c022e1-a42b-5c5e-7cae-ab14c1aa86ba@redhat.com> Date: Thu, 30 Apr 2020 13:24:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 04/30/20 03:18, Gao, Liming wrote: > Laszlo: > > > Thanks > Liming >> -----Original Message----- >> From: Laszlo Ersek >> Sent: Thursday, April 30, 2020 2:04 AM >> To: Sean Brogan ; devel@edk2.groups.io; michael.kubacki@outlook.com >> Cc: Andrew Fish ; Ard Biesheuvel ; Bret Barkelew ; >> Justen, Jordan L ; Leif Lindholm ; Gao, Liming ; Kinney, >> Michael D ; Ni, Ray ; Sean Brogan >> Subject: Re: [edk2-devel] [PATCH v3 0/7] Edk2 Platform and Core CI for ArmVirtPkg, EmulatorPkg, and OvmfPkg >> >> On 04/28/20 18:35, Sean Brogan wrote: >>> I think this was my fault. >>> >>> I was under the impression that a patch needed one of developers listed >>> in the (m) or (r) section of maintainers.txt to provide a reviewed-by. >>> My new understanding is an ack from the (m) plus anyone providing a >>> reviewed-by is enough. >> >> It depends on the maintainer, too. >> >> Personally I give R-b if I carefully review the patch and am pleased >> with it. >> >> I give A-b if I review the patch for general sanity, but don't dig into >> the details. I can also give A-b if someone I trust to do a good review >> in the subject technical area provides an R-b, regardless of whether >> they are an "R" or an otherwise un-designated contributor. With "R" >> folks the chance is higher for me to see such an R-b posted in the first >> place, of course. >> >> I do think an "M" person should provide "at least" an A-b, even if they >> delegate the actual detailed review to someone else. >> > I don't think there is such requirement to maintainer now. If you think this is required, > You can give the proposal to add this requirement in Maintainers.txt. I feel that the current language is expressive enough: M: Package Maintainer: Cc address for patches and questions. Responsible for reviewing and pushing package changes to source control. R: Package Reviewer: Cc address for patches and questions. Reviewers help maintainers review code, but don't have push access. A designated Package Reviewer is reasonably familiar with the Package (or some modules thereof), and/or provides testing or regression testing for the Package (or some modules thereof), in certain platforms and environments. See "Responsible for reviewing" vs "help [...] review". NB, I don't want to force other maintainers to ACK explicitly, if they consider their co-reviewers' feedback definitive / authoritative. It's just that, when *only* "R" approval has been posted, and the "M" folks with jurisdiction don't react at all (no feedback, also not merging the patch), a *different* maintainer that wants to help out may not be able to decide whether he/she should wait for more ("M") feedback, or just run with the "R" feedback. An explicit ACK from the owner "M" guys clears this up at once. Thanks Laszlo