On 04/30/20 03:18, Gao, Liming wrote: > Laszlo: > > > Thanks > Liming >> -----Original Message----- >> From: Laszlo Ersek <ler...@redhat.com> >> Sent: Thursday, April 30, 2020 2:04 AM >> To: Sean Brogan <spbro...@outlook.com>; devel@edk2.groups.io; >> michael.kuba...@outlook.com >> Cc: Andrew Fish <af...@apple.com>; Ard Biesheuvel <ard.biesheu...@arm.com>; >> Bret Barkelew <bret.barke...@microsoft.com>; >> Justen, Jordan L <jordan.l.jus...@intel.com>; Leif Lindholm >> <l...@nuviainc.com>; Gao, Liming <liming....@intel.com>; Kinney, >> Michael D <michael.d.kin...@intel.com>; Ni, Ray <ray...@intel.com>; Sean >> Brogan <sean.bro...@microsoft.com> >> 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#58431): https://edk2.groups.io/g/devel/message/58431 Mute This Topic: https://groups.io/mt/73251653/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-