BaseTools/Scripts/GetMaintainer.py | 43 +++++++++++++++++++----------- 1 file changed, 27 insertions(+), 16 deletions(-)
OK, so this a bit of a backwards review, but I figured I might as well show how I would prefer the split. I'm adding a patch that changes internal returns to dictionaries instead of multiple return values. There are no functional differences between the original submission and this for 1-2,4-5/5, so I'm happy to give Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com> for those. 3/5 is new and requires review by someone else. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593 Fix logic bug where maintainers was incorrectly added to lists. If a package only has reviewers and no maintainers, then also return the <default> maintainers. In order to detect this case, get_maintainers() is updated to return maintainers, reviews, and lists separately instead of a single merged list. This also allows this module to be used by other scripts that need to distinguish between maintainers, reviewers, and lists. Simplify logic that accumulates maintainers, reviewers, lists. Sort the list of output addresses alphabetically and use set() instead of OrderedDict() to accumulate unique addresses. Changes since v2: - Reworked internal return logic to use dictionaries. - Reordered cleanup before new functionality. Changes since v1: - Split into patch series Cc: Rebecca Cran <rebecca@bsdio.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Bob Feng <bob.c.feng@intel.com> Cc: Yuwei Chen <yuwei.chen@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Leif Lindholm (1): BaseTools/Scripts/GetMaintainer: refactor internal returns as dicts Michael D Kinney (4): BaseTools/Scripts/GetMaintainer: Fix logic bug collecting maintainers BaseTools/Scripts/GetMaintainer: Simplify logic BaseTools/Scripts/GetMaintainer: Handle reviewer only case BaseTools/Scripts/GetMaintainer: Sort output addresses BaseTools/Scripts/GetMaintainer.py | 43 +++++++++++++++++++----------- 1 file changed, 27 insertions(+), 16 deletions(-) -- 2.39.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111063): https://edk2.groups.io/g/devel/message/111063 Mute This Topic: https://groups.io/mt/102513765/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
For the series: Acked-by: Rebecca Cran <rebecca@bsdio.com> On 11/10/23 12:30, Leif Lindholm wrote: > OK, so this a bit of a backwards review, but I figured I might as > well show how I would prefer the split. I'm adding a patch that > changes internal returns to dictionaries instead of multiple return > values. > > There are no functional differences between the original submission > and this for 1-2,4-5/5, so I'm happy to give > Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com> > for those. 3/5 is new and requires review by someone else. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593 > > Fix logic bug where maintainers was incorrectly added to lists. > > If a package only has reviewers and no maintainers, then also > return the <default> maintainers. > > In order to detect this case, get_maintainers() is updated to > return maintainers, reviews, and lists separately instead of > a single merged list. This also allows this module to be used > by other scripts that need to distinguish between maintainers, > reviewers, and lists. > > Simplify logic that accumulates maintainers, reviewers, lists. > > Sort the list of output addresses alphabetically and use set() > instead of OrderedDict() to accumulate unique addresses. > > Changes since v2: > - Reworked internal return logic to use dictionaries. > - Reordered cleanup before new functionality. > Changes since v1: > - Split into patch series > > Cc: Rebecca Cran <rebecca@bsdio.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Yuwei Chen <yuwei.chen@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Leif Lindholm (1): > BaseTools/Scripts/GetMaintainer: refactor internal returns as dicts > > Michael D Kinney (4): > BaseTools/Scripts/GetMaintainer: Fix logic bug collecting maintainers > BaseTools/Scripts/GetMaintainer: Simplify logic > BaseTools/Scripts/GetMaintainer: Handle reviewer only case > BaseTools/Scripts/GetMaintainer: Sort output addresses > > BaseTools/Scripts/GetMaintainer.py | 43 +++++++++++++++++++----------- > 1 file changed, 27 insertions(+), 16 deletions(-) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111070): https://edk2.groups.io/g/devel/message/111070 Mute This Topic: https://groups.io/mt/102513765/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Merged: https://github.com/tianocore/edk2/pull/5033 > -----Original Message----- > From: Rebecca Cran <rebecca@bsdio.com> > Sent: Friday, November 10, 2023 12:36 PM > To: Leif Lindholm <quic_llindhol@quicinc.com>; devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming > <gaoliming@byosoft.com.cn>; Feng, Bob C <bob.c.feng@intel.com>; Chen, > Christine <yuwei.chen@intel.com> > Subject: Re: [PATCH v3 0/5] BaseTools/Scripts/GetMaintainer: Handle > reviewer only case > > For the series: > > Acked-by: Rebecca Cran <rebecca@bsdio.com> > > > On 11/10/23 12:30, Leif Lindholm wrote: > > OK, so this a bit of a backwards review, but I figured I might as > > well show how I would prefer the split. I'm adding a patch that > > changes internal returns to dictionaries instead of multiple return > > values. > > > > There are no functional differences between the original submission > > and this for 1-2,4-5/5, so I'm happy to give > > Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com> > > for those. 3/5 is new and requires review by someone else. > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593 > > > > Fix logic bug where maintainers was incorrectly added to lists. > > > > If a package only has reviewers and no maintainers, then also > > return the <default> maintainers. > > > > In order to detect this case, get_maintainers() is updated to > > return maintainers, reviews, and lists separately instead of > > a single merged list. This also allows this module to be used > > by other scripts that need to distinguish between maintainers, > > reviewers, and lists. > > > > Simplify logic that accumulates maintainers, reviewers, lists. > > > > Sort the list of output addresses alphabetically and use set() > > instead of OrderedDict() to accumulate unique addresses. > > > > Changes since v2: > > - Reworked internal return logic to use dictionaries. > > - Reordered cleanup before new functionality. > > Changes since v1: > > - Split into patch series > > > > Cc: Rebecca Cran <rebecca@bsdio.com> > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Bob Feng <bob.c.feng@intel.com> > > Cc: Yuwei Chen <yuwei.chen@intel.com> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > > > Leif Lindholm (1): > > BaseTools/Scripts/GetMaintainer: refactor internal returns as > dicts > > > > Michael D Kinney (4): > > BaseTools/Scripts/GetMaintainer: Fix logic bug collecting > maintainers > > BaseTools/Scripts/GetMaintainer: Simplify logic > > BaseTools/Scripts/GetMaintainer: Handle reviewer only case > > BaseTools/Scripts/GetMaintainer: Sort output addresses > > > > BaseTools/Scripts/GetMaintainer.py | 43 +++++++++++++++++++------- > ---- > > 1 file changed, 27 insertions(+), 16 deletions(-) > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111111): https://edk2.groups.io/g/devel/message/111111 Mute This Topic: https://groups.io/mt/102513765/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.