[edk2-devel] [PATCH v3 0/5] BaseTools/Scripts/GetMaintainer: Handle reviewer only case

[edk2-devel] [PATCH v3 0/5] BaseTools/Scripts/GetMaintainer: Handle reviewer only case

[Leif Lindholm]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 1/5] BaseTools/Scripts/GetMaintainer: Fix logic bug collecting maintainers

[Leif Lindholm]

From: Michael D Kinney <michael.d.kinney@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593

Fix logic bug where maintainers is incorrectly added to lists.

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: Leif Lindholm <quic_llindhol@quicinc.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Scripts/GetMaintainer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index d1e042c0afe4..2a3147c059b5 100644
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -88,7 +88,7 @@ def get_section_maintainers(path, section):
             if isinstance(address, list):
                 maintainers += address
             else:
-                lists += [address]
+                maintainers += [address]
         for address in section['list']:
             # Convert to list if necessary
             if isinstance(address, list):
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111064): https://edk2.groups.io/g/devel/message/111064
Mute This Topic: https://groups.io/mt/102513767/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 2/5] BaseTools/Scripts/GetMaintainer: Simplify logic

[Leif Lindholm]

From: Michael D Kinney <michael.d.kinney@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593

get_section_maintainers() either returns a list with
valid entries or an empty list.  It never returns None.
Simplify logic that accumulates maintainers and lists by
unconditionally appending lists returned from
get_section_maintainers().

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: Leif Lindholm <quic_llindhol@quicinc.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Scripts/GetMaintainer.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index 2a3147c059b5..cdcdff750635 100644
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -105,10 +105,8 @@ def get_maintainers(path, sections, level=0):
     lists = []
     for section in sections:
         tmp_maint, tmp_lists = get_section_maintainers(path, section)
-        if tmp_maint:
-            maintainers += tmp_maint
-        if tmp_lists:
-            lists += tmp_lists
+        maintainers += tmp_maint
+        lists += tmp_lists
 
     if not maintainers:
         # If no match found, look for match for (nonexistent) file
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111065): https://edk2.groups.io/g/devel/message/111065
Mute This Topic: https://groups.io/mt/102513768/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 3/5] BaseTools/Scripts/GetMaintainer: refactor internal returns as dicts

[Leif Lindholm]

To clean up interfaces, change the lookup functions to return dictionaries
rather than multiple values.

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>
Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
---
 BaseTools/Scripts/GetMaintainer.py | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index cdcdff750635..cb3aadbbefb1 100644
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -96,7 +96,7 @@ def get_section_maintainers(path, section):
             else:
                 lists += [address]
 
-    return maintainers, lists
+    return {'maintainers': maintainers, 'lists': lists}
 
 def get_maintainers(path, sections, level=0):
     """For 'path', iterates over all sections, returning maintainers
@@ -104,22 +104,24 @@ def get_maintainers(path, sections, level=0):
     maintainers = []
     lists = []
     for section in sections:
-        tmp_maint, tmp_lists = get_section_maintainers(path, section)
-        maintainers += tmp_maint
-        lists += tmp_lists
+        recipients = get_section_maintainers(path, section)
+        maintainers += recipients['maintainers']
+        lists += recipients['lists']
 
     if not maintainers:
         # If no match found, look for match for (nonexistent) file
         # REPO.working_dir/<default>
         print('"%s": no maintainers found, looking for default' % path)
         if level == 0:
-            maintainers = get_maintainers('<default>', sections, level=level + 1)
+            recipients = get_maintainers('<default>', sections, level=level + 1)
+            maintainers += recipients['maintainers']
+            lists += recipients['lists']
         else:
             print("No <default> maintainers set for project.")
         if not maintainers:
             return None
 
-    return maintainers + lists
+    return {'maintainers': maintainers, 'lists': lists}
 
 def parse_maintainers_line(line):
     """Parse one line of Maintainers.txt, returning any match group and its key."""
@@ -184,9 +186,8 @@ if __name__ == '__main__':
 
     for file in FILES:
         print(file)
-        addresslist = get_maintainers(file, SECTIONS)
-        if addresslist:
-            ADDRESSES += addresslist
+        recipients = get_maintainers(file, SECTIONS)
+        ADDRESSES += recipients['maintainers'] + recipients['lists']
 
     for address in list(OrderedDict.fromkeys(ADDRESSES)):
         if '<' in address and '>' in address:
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111066): https://edk2.groups.io/g/devel/message/111066
Mute This Topic: https://groups.io/mt/102513771/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 4/5] BaseTools/Scripts/GetMaintainer: Handle reviewer only case

[Leif Lindholm]

From: Michael D Kinney <michael.d.kinney@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593

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.

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: Leif Lindholm <quic_llindhol@quicinc.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Scripts/GetMaintainer.py | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index cb3aadbbefb1..1361fb6c0e30 100644
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -76,6 +76,7 @@ def get_section_maintainers(path, section):
     """Returns a list with email addresses to any M: and R: entries
        matching the provided path in the provided section."""
     maintainers = []
+    reviewers = []
     lists = []
     nowarn_status = ['Supported', 'Maintained']
 
@@ -83,12 +84,18 @@ def get_section_maintainers(path, section):
         for status in section['status']:
             if status not in nowarn_status:
                 print('WARNING: Maintained status for "%s" is \'%s\'!' % (path, status))
-        for address in section['maintainer'], section['reviewer']:
+        for address in section['maintainer']:
             # Convert to list if necessary
             if isinstance(address, list):
                 maintainers += address
             else:
                 maintainers += [address]
+        for address in section['reviewer']:
+            # Convert to list if necessary
+            if isinstance(address, list):
+                reviewers += address
+            else:
+                reviewers += [address]
         for address in section['list']:
             # Convert to list if necessary
             if isinstance(address, list):
@@ -96,16 +103,18 @@ def get_section_maintainers(path, section):
             else:
                 lists += [address]
 
-    return {'maintainers': maintainers, 'lists': lists}
+    return {'maintainers': maintainers, 'reviewers': reviewers, 'lists': lists}
 
 def get_maintainers(path, sections, level=0):
     """For 'path', iterates over all sections, returning maintainers
        for matching ones."""
     maintainers = []
+    reviewers = []
     lists = []
     for section in sections:
         recipients = get_section_maintainers(path, section)
         maintainers += recipients['maintainers']
+        reviewers += recipients['reviewers']
         lists += recipients['lists']
 
     if not maintainers:
@@ -115,13 +124,14 @@ def get_maintainers(path, sections, level=0):
         if level == 0:
             recipients = get_maintainers('<default>', sections, level=level + 1)
             maintainers += recipients['maintainers']
+            reviewers += recipients['reviewers']
             lists += recipients['lists']
         else:
             print("No <default> maintainers set for project.")
         if not maintainers:
             return None
 
-    return {'maintainers': maintainers, 'lists': lists}
+    return {'maintainers': maintainers, 'reviewers': reviewers, 'lists': lists}
 
 def parse_maintainers_line(line):
     """Parse one line of Maintainers.txt, returning any match group and its key."""
@@ -187,7 +197,7 @@ if __name__ == '__main__':
     for file in FILES:
         print(file)
         recipients = get_maintainers(file, SECTIONS)
-        ADDRESSES += recipients['maintainers'] + recipients['lists']
+        ADDRESSES += recipients['maintainers'] + recipients['reviewers'] + recipients['lists']
 
     for address in list(OrderedDict.fromkeys(ADDRESSES)):
         if '<' in address and '>' in address:
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111067): https://edk2.groups.io/g/devel/message/111067
Mute This Topic: https://groups.io/mt/102513772/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 5/5] BaseTools/Scripts/GetMaintainer: Sort output addresses

[Leif Lindholm]

From: Michael D Kinney <michael.d.kinney@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4593

Sort the list of output addresses alphabetically so this
script produces the same output even if the order of patches
in a patch series is modified such that that order of files
processed by this script changes.

Use set() logic instead of OrderedDict to accumulate the
list of unique addresses that are sorted alphabetically.

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: Leif Lindholm <quic_llindhol@quicinc.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 BaseTools/Scripts/GetMaintainer.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index 1361fb6c0e30..8097ba4e7bd6 100644
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -192,14 +192,16 @@ if __name__ == '__main__':
     else:
         FILES = get_modified_files(REPO, ARGS)
 
-    ADDRESSES = []
-
+    # Accumulate a sorted list of addresses
+    ADDRESSES = set([])
     for file in FILES:
         print(file)
         recipients = get_maintainers(file, SECTIONS)
-        ADDRESSES += recipients['maintainers'] + recipients['reviewers'] + recipients['lists']
+        ADDRESSES |= set(recipients['maintainers'] + recipients['reviewers'] + recipients['lists'])
+    ADDRESSES = list(ADDRESSES)
+    ADDRESSES.sort()
 
-    for address in list(OrderedDict.fromkeys(ADDRESSES)):
+    for address in ADDRESSES:
         if '<' in address and '>' in address:
             address = address.split('>', 1)[0] + '>'
         print('  %s' % address)
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111068): https://edk2.groups.io/g/devel/message/111068
Mute This Topic: https://groups.io/mt/102513774/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 3/5] BaseTools/Scripts/GetMaintainer: refactor internal returns as dicts

[Michael D Kinney]

Hi Leif,

Thank you for the addition cleanup.

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: Friday, November 10, 2023 11:31 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Rebecca Cran
> <rebecca@bsdio.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Feng, Bob
> C <bob.c.feng@intel.com>; Chen, Christine <yuwei.chen@intel.com>
> Subject: [PATCH v3 3/5] BaseTools/Scripts/GetMaintainer: refactor
> internal returns as dicts
> 
> To clean up interfaces, change the lookup functions to return
> dictionaries
> rather than multiple values.
> 
> 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>
> Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
> ---
>  BaseTools/Scripts/GetMaintainer.py | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/BaseTools/Scripts/GetMaintainer.py
> b/BaseTools/Scripts/GetMaintainer.py
> index cdcdff750635..cb3aadbbefb1 100644
> --- a/BaseTools/Scripts/GetMaintainer.py
> +++ b/BaseTools/Scripts/GetMaintainer.py
> @@ -96,7 +96,7 @@ def get_section_maintainers(path, section):
>              else:
>                  lists += [address]
> 
> -    return maintainers, lists
> +    return {'maintainers': maintainers, 'lists': lists}
> 
>  def get_maintainers(path, sections, level=0):
>      """For 'path', iterates over all sections, returning maintainers
> @@ -104,22 +104,24 @@ def get_maintainers(path, sections, level=0):
>      maintainers = []
>      lists = []
>      for section in sections:
> -        tmp_maint, tmp_lists = get_section_maintainers(path, section)
> -        maintainers += tmp_maint
> -        lists += tmp_lists
> +        recipients = get_section_maintainers(path, section)
> +        maintainers += recipients['maintainers']
> +        lists += recipients['lists']
> 
>      if not maintainers:
>          # If no match found, look for match for (nonexistent) file
>          # REPO.working_dir/<default>
>          print('"%s": no maintainers found, looking for default' %
> path)
>          if level == 0:
> -            maintainers = get_maintainers('<default>', sections,
> level=level + 1)
> +            recipients = get_maintainers('<default>', sections,
> level=level + 1)
> +            maintainers += recipients['maintainers']
> +            lists += recipients['lists']
>          else:
>              print("No <default> maintainers set for project.")
>          if not maintainers:
>              return None
> 
> -    return maintainers + lists
> +    return {'maintainers': maintainers, 'lists': lists}
> 
>  def parse_maintainers_line(line):
>      """Parse one line of Maintainers.txt, returning any match group
> and its key."""
> @@ -184,9 +186,8 @@ if __name__ == '__main__':
> 
>      for file in FILES:
>          print(file)
> -        addresslist = get_maintainers(file, SECTIONS)
> -        if addresslist:
> -            ADDRESSES += addresslist
> +        recipients = get_maintainers(file, SECTIONS)
> +        ADDRESSES += recipients['maintainers'] + recipients['lists']
> 
>      for address in list(OrderedDict.fromkeys(ADDRESSES)):
>          if '<' in address and '>' in address:
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111069): https://edk2.groups.io/g/devel/message/111069
Mute This Topic: https://groups.io/mt/102513771/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/5] BaseTools/Scripts/GetMaintainer: Handle reviewer only case

[Rebecca Cran]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/5] BaseTools/Scripts/GetMaintainer: Handle reviewer only case

[Michael D Kinney]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-