From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web10.7222.1588254037402591618 for ; Thu, 30 Apr 2020 06:40:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=zAJ6bJyF; spf=pass (domain: nuviainc.com, ip: 209.85.221.67, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f67.google.com with SMTP id b11so6958117wrs.6 for ; Thu, 30 Apr 2020 06:40:37 -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=ae4/9osADlJEA9CaNHwzx40x/xN7S0CS2zkj6EVo/tc=; b=zAJ6bJyFnts4+pwHqVeLV79hCXbNKkqKFZkdrcQRWn8QnMO1Imef0WIyrQOodhWrf3 pEg/g0ARoTgaLAkAw8KPdI73YqTzTlDaICnjgWsHsB0s6cTB1QQzc8YVkEZdBXhvcISD MZYIy4UC0OuKAchMUOlOOSchAufKvSv+KZW/Svdhaoc/W/YAHhAiWSRSx8640ca0+TLs L0Bvgw9QcF05Tn+qTmgMn3fQ1B2DUMoj6pcFcEH1kMU+/glRLU6cCforLni1g1t0lqZn BiV+S0P335OJiYfYmrAp8LsFRXDTW9L3OzCBs+JG4SA8sbnQhUKemB/pMQXwbiNUTG9I WHHQ== 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=ae4/9osADlJEA9CaNHwzx40x/xN7S0CS2zkj6EVo/tc=; b=VB0Npqy1HBtQxgxlXqQw4JtUN1mnLg4ue8OI7J9Ni7ZR3V8zAGfto//Rju5cyMZhgM yTEL6elvFDYrxEu4EEUNka0ilhAXO6oTyI+DQgxuDQarwhbuA2Y8dD7wNI80racTwsoh o3qnIUpC0UpCjLhbJmUF0WElRPOJG9xCU3c+j4aqTKzn2Wo0ByLy4B45/OpCVAV6WnQq ely+yVlnYSxCpcgEIOxuteR0Q+t7uPW/PM22X0EfR4j3hskSfO+0SIoYoBGbTF2sIYZl N/v0BoR/+ldI9szlTh56PvBpFj/ZNXepYCogOKyysAxhStuRjZae7eyDohPp8OeNpE1S QXlg== X-Gm-Message-State: AGi0PuYqjkHKIGhDeJuYUZORM+SPtj+/C4tt/dOP6ubqryhmJoLfggBS HC3Yj8MzAHleoYx0pt55AH01p2cCwMM94PpqsyeNnPW/MFf5dl3mAFpp4DMwi8CQMhwupQNGFEc b5R7XZgyEPEEmzw3vKNUamuZt3qCAghjoWuheiITt9z+MwZdUqOMR7MffGcGBNq4= X-Google-Smtp-Source: APiQypLcT6HIa/7t6C3yxqipGDbBhvBkpfrHvg54xOlL8i4jsgB8vBfv8Zk4nXQlp5EqBuvJrNavRA== X-Received: by 2002:adf:ce0a:: with SMTP id p10mr3851260wrn.89.1588254035625; Thu, 30 Apr 2020 06:40:35 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id v131sm12897412wmb.19.2020.04.30.06.40.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2020 06:40:35 -0700 (PDT) Date: Thu, 30 Apr 2020 14:40:32 +0100 From: "Leif Lindholm" To: devel@edk2.groups.io, lersek@redhat.com Cc: "Gao, Liming" , Sean Brogan , "michael.kubacki@outlook.com" , Andrew Fish , Ard Biesheuvel , Bret Barkelew , "Justen, Jordan L" , "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 Message-ID: <20200430134032.GT21486@vanye> References: <38d73ec4-ce01-d6a6-c2dd-5e925afa2084@redhat.com> <16c022e1-a42b-5c5e-7cae-ab14c1aa86ba@redhat.com> MIME-Version: 1.0 In-Reply-To: <16c022e1-a42b-5c5e-7cae-ab14c1aa86ba@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Apr 30, 2020 at 13:24:57 +0200, Laszlo Ersek wrote: > 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. Agreed. As a (probably unnexessary) counterpoint, I'll just add that in instances where the Reviewer clearly is better placed to determine what is correct and the Maintainer is mainly driving the bus, it could be misleading for the Maintainer to add an A-b (whereas actually pushing the patch is a very clear explicit approval). / Leif