[RFC PATCH 2/3] Maintainers.txt: add wildcard path association for Arm/AArch64


Wu, Hao A
 

-----Original Message-----
From: Wu, Hao A
Sent: Wednesday, July 03, 2019 3:53 PM
To: 'Leif Lindholm'
Cc: devel@edk2.groups.io
Subject: RE: [RFC PATCH 2/3] Maintainers.txt: add wildcard path association
for Arm/AArch64

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@...]
Sent: Saturday, June 15, 2019 4:21 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C; Gao, Liming; Andrew Fish; Laszlo Ersek; Kinney, Michael D;
Wu, Hao A
Subject: [RFC PATCH 2/3] Maintainers.txt: add wildcard path association for
Arm/AArch64

Add Ard and Leif as responsible for any path matching
F: */Arm/
F: */AArch64/

Signed-off-by: Leif Lindholm <leif.lindholm@...>
---
Maintainers.txt | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index cd32f9b00170..e415f51468d5 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -82,6 +82,14 @@ EDK II Releases:
W: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-
Planning
M: Liming Gao <liming.gao@...>

+EDK II Architectures:
+---------------------
+ARM, AARCH64
+F: */AArch64/
+F: */Arm/

Hello Leif,

I want to confirm a couple of usage model for the wildcard character.

Maybe they have been explained with your response to Laszlo's comments,
but I just want to double-confirm with my using cases.

1. Matching multiple levels of directories

For the below 2 folders:
MdeModulePkg/Bus/Ufs/
MdeModulePkg/Bus/Pci/UfsPciHcDxe/

I can use:
MdeModulePkg/*Ufs*/

to match them all, right?


2. Matching header files (usually within the Include/ directory)

In some cases, a feature may include some drivers + some include header
files. So for the below case:
MdeModulePkg/Include/Library/HiiLib.h
MdeModulePkg/Include/Guid/HiiResourceSampleHii.h
MdeModulePkg/Universal/HiiDatabaseDxe/
MdeModulePkg/Universal/HiiResourcesSampleDxe/
MdeModulePkg/Library/UefiHiiLib/

Should I use:
MdeModulePkg/*Hii*/

to match the drivers(libraries) and the headers all or should I use:
MdeModulePkg/*Hii*/
MdeModulePkg/Include/*Hii*.h

instead?

Best Regards,
Hao Wu

Posting to the rfc list as well.

Best Regards,
Hao Wu




+M: Leif Lindholm <leif.lindholm@...>
+M: Ard Biesheuvel <ard.biesheuvel@...>
+
EDK II Packages:
----------------
ArmPkg
--
2.11.0


Wu, Hao A
 

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@...]
Sent: Wednesday, July 03, 2019 6:44 PM
To: Wu, Hao A
Cc: devel@edk2.groups.io
Subject: Re: [RFC PATCH 2/3] Maintainers.txt: add wildcard path association
for Arm/AArch64

On Wed, Jul 03, 2019 at 07:52:46AM +0000, Wu, Hao A wrote:
-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@...]
Sent: Saturday, June 15, 2019 4:21 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C; Gao, Liming; Andrew Fish; Laszlo Ersek; Kinney, Michael
D;
Wu, Hao A
Subject: [RFC PATCH 2/3] Maintainers.txt: add wildcard path association
for
Arm/AArch64

Add Ard and Leif as responsible for any path matching
F: */Arm/
F: */AArch64/

Signed-off-by: Leif Lindholm <leif.lindholm@...>
---
Maintainers.txt | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index cd32f9b00170..e415f51468d5 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -82,6 +82,14 @@ EDK II Releases:
W: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
Release-
Planning
M: Liming Gao <liming.gao@...>

+EDK II Architectures:
+---------------------
+ARM, AARCH64
+F: */AArch64/
+F: */Arm/

Hello Leif,

I want to confirm a couple of usage model for the wildcard character.

Maybe they have been explained with your response to Laszlo's comments,
but I just want to double-confirm with my using cases.

1. Matching multiple levels of directories

For the below 2 folders:
MdeModulePkg/Bus/Ufs/
MdeModulePkg/Bus/Pci/UfsPciHcDxe/

I can use:
MdeModulePkg/*Ufs*/

to match them all, right?
Yes, that is how it currently works (I think?). It is unclear to me if
there is consensus on how we want it to work in the future.

Yes, the proposed BaseTools script is working for:
MdeModulePkg/*Ufs*/

to match changes made in both directories.

I am also fine (and prefer) for the wildcard character to match multiple
levels of folders.



2. Matching header files (usually within the Include/ directory)

In some cases, a feature may include some drivers + some include header
files. So for the below case:
MdeModulePkg/Include/Library/HiiLib.h
MdeModulePkg/Include/Guid/HiiResourceSampleHii.h
MdeModulePkg/Universal/HiiDatabaseDxe/
MdeModulePkg/Universal/HiiResourcesSampleDxe/
MdeModulePkg/Library/UefiHiiLib/

Should I use:
MdeModulePkg/*Hii*/

to match the drivers(libraries) and the headers all or should I use:
MdeModulePkg/*Hii*/
MdeModulePkg/Include/*Hii*.h
My view is that this second option would be required.
If MdeModulePkg/*Hii*/ matches on the .h files, that is effectively a
bug - is that the behaviour you are currently seeing?

The script is working like the 2nd option, and I think it is expected.
So, there seems to me no bug in my tests.


I do met a little problem that for a patch that touches multiple features,
the contacts order in the output is not ideally perfect:

Content in Maintainers.txt:
'''
MdeModulePkg
F: MdeModulePkg/
W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
M: Jian J Wang <jian.j.wang@...>
M: Hao A Wu <hao.a.wu@...>

F: MdeModulePkg/*Ufs*/
R: Ufs Guy <ufs@...>

F: MdeModulePkg/Include/*Hii*.h
F: MdeModulePkg/*Hii*/
R: Hii Guy <hii@...>
'''

Script output:
'''
$ py BaseTools/Scripts/GetMaintainer.py HEAD
MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.uni
Jian J Wang <jian.j.wang@...>
Hao A Wu <hao.a.wu@...>
Ufs Guy <ufs@...>
devel@edk2.groups.io
Hii Guy <hii@...>
'''

The 'Hii Guy' is at the end of the list.

But this is a rather minor problem and does not impact the use at all.

Best Regards,
Hao Wu


Best Regards,

Leif


Leif Lindholm <leif.lindholm@...>
 

On Thu, Jul 04, 2019 at 06:14:04AM +0000, Wu, Hao A wrote:
I do met a little problem that for a patch that touches multiple features,
the contacts order in the output is not ideally perfect:

Content in Maintainers.txt:
'''
MdeModulePkg
F: MdeModulePkg/
W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
M: Jian J Wang <jian.j.wang@...>
M: Hao A Wu <hao.a.wu@...>

F: MdeModulePkg/*Ufs*/
R: Ufs Guy <ufs@...>

F: MdeModulePkg/Include/*Hii*.h
F: MdeModulePkg/*Hii*/
R: Hii Guy <hii@...>
'''

Script output:
'''
$ py BaseTools/Scripts/GetMaintainer.py HEAD
MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.uni
Jian J Wang <jian.j.wang@...>
Hao A Wu <hao.a.wu@...>
Ufs Guy <ufs@...>
devel@edk2.groups.io
Hii Guy <hii@...>
'''

The 'Hii Guy' is at the end of the list.
Totally agree. The idea was to keep the first revision simple.
Another thing missing (that I would argue is even more important than
listed order), is an indication of *why* said person is listed.

But this is a rather minor problem and does not impact the use at all.
Good. I will respin the set based on Laszlo's feedback on previous
version and resubmit.

Best Regards,

Leif