Re: [edk2-devel] [PATCH edk2-platforms v2 1/1] Move to the GitHub Pull Request workflow

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On 2024-08-01 13:49, Rebecca Cran wrote:
> Replies inline.
> 
> On 8/1/24 3:45 AM, Leif Lindholm wrote:
>> On Sat, Jul 27, 2024 at 14:31:56 -0600, Rebecca Cran wrote:
>>> Migrate data from Maintainers.txt to the GitHub standard CODEOWNERS
>>> files plus REVIEWERS and CONTRIBUTORS.md. The latter file contains
>>> mappings from name to email address and GitHub usernames, which will
>>> help people who want to email maintainers instead of using GitHub.
>>>
>>> Add .github/workflows/AssignReviewers.yml which adds reviewers to a
>>> Pull Request based on the content of the REVIEWERS file.
>>>
>>> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
>>> ---
>>>   .github/workflows/AssignReviewers.yml |  28 ++
>>>   CODEOWNERS                            | 146 +++++++
>>>   CONTRIBUTORS.md                       |  68 +++
>>>   Maintainers.txt                       | 445 --------------------
>>>   REVIEWERS                             |  92 ++++
>>>   Readme.md                             |  32 +-
>>>   6 files changed, 361 insertions(+), 450 deletions(-)
>>>
>>> diff --git a/.github/workflows/AssignReviewers.yml 
>>> b/.github/workflows/AssignReviewers.yml
>>> new file mode 100644
>>> index 000000000000..8ee95edbb2c1
>>> --- /dev/null
>>> +++ b/.github/workflows/AssignReviewers.yml
>>> @@ -0,0 +1,28 @@
>>> +## @file
>>> +# Assign reviewers from a REVIEWERS file using CODEOWNERS syntax
>> If we're starting to do manual copying around of files between
>> repositories, can we add a manual audit trail?
>>
>> I.e., full URL of repository, path inside repository, and commit hash?
>>
>> I think my preference would be in this file header.
> 
> I'm not sure I understand. This file came from 
> https://github.com/mdkinney/github-action-assign-reviewers/blob/main/.github/workflows/AssignReviewers.yml.
> 
> Would you like me to add a note to .github/workflows/AssignReviewers.yml 
> to say that's where it came from?

I'd like to see it added *somewhere*. Where it came from and the (short) 
commit hash at the version you picked.

If we add it here, then it becomes immediately visible as something to 
update if someone later on copies a newer version on top of it.

Ultimately we might want to add things like this to a common repository 
either directly referenced by CI/codeforge or imported as submodules, so 
that we aren't needing to copy code arounbd between repositories.
But that's not for this patch to resolve.

>>> diff --git a/CODEOWNERS b/CODEOWNERS
>>> new file mode 100644
>>> index 000000000000..bc86dd113398
>>> --- /dev/null
>>> +++ b/CODEOWNERS
>>> @@ -0,0 +1,146 @@
>>> ...
>>> +# Sophgo platforms and silicon
>>> +/Platform/Sophgo/** @vlsunil
>>> +/Silicon/Sophgo/SG2042Pkg/** @vlsunil
>> Not super important, but if we're doing this change it would be an
>> opportunity to re-sort the areas alphabetically, since that broke
>> somewhere along the way.
> 
> Fixed.

Thanks!

>>> diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
>>> new file mode 100644
>>> index 000000000000..84882bcab2fa
>>> --- /dev/null
>>> +++ b/CONTRIBUTORS.md
>>> @@ -0,0 +1,68 @@
>>> +EDK II Platforms Maintainers and Reviewers
>>> +=========================================================
>>> +
>>> +This file provides information about the people who maintain and review
>>> +code for EDK II Platforms. For information about who from this file
>>> +maintains (i.e. owns and can commit changes) and who reviews changes in
>>> +various parts of the repo, see the CODEOWNERS and REVIEWERS files.
>>> +
>>> +| Name                   | e-mail address                   | Github 
>>> username                                      |
>>> +|------------------------|----------------------------------|------------------------------------------------------|
>>> +| Leif Lindholm          | quic_llindhol@quicinc.com        | 
>>> [@leiflindholm](https://github.com/leiflindholm)     |
...
>>> +| Marvin H??user          | mhaeuser@posteo.de               | 
>>> [@mhaeuser](https://github.com/mhaeuser)             |
>> Is this charset corruption only in the email?
 >
> Yes. See 
> https://github.com/bcran/edk2-platforms/blob/github-pr/CONTRIBUTORS.md .

Cool, no issue then.

>>> +| Sai Chaganty           | rangasai.v.chaganty@intel.com    | 
...
>>> +| USER0FISH              | libing1202@outlook.com           | 
>>> [@USER0FISH](https://github.com/USER0FISH)           |
>> Likewise, could we sort this alphabetically by name string?
>> (If that's annoying, can you push it to a branch where I could pull it
>> down and sort it?)
> 
> Done. Took about 30 seconds (column select mode, copy lines to a file, 
> run through `sort`) :)

Excellent, thanks!

>>> diff --git a/Maintainers.txt b/Maintainers.txt
>>> deleted file mode 100644
>>> index 824838486072..000000000000
>>> --- a/Maintainers.txt
>>> +++ /dev/null
>>> @@ -1,445 +0,0 @@
>>> ,,,
>>> -Any contributions to this branch should be submitted via email to the
>>> -edk2-devel mailing list with a subject prefix of `[platforms]`. See
>>> -[Laszlo's excellent 
>>> guide](https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers) for details
>>> -on how to do this successfully.
>>> +Any contributions to this branch should be submitted via GitHub Pull 
>>> Request,
>>> +or email to the edk2-devel mailing list with a subject prefix of 
>>> `[platforms]`. See
>>> +[Laszlo's excellent 
>>> guide](https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers) for details on how to do this successfully.
>> Can you put back this line break?
> Fixed.
> 
>>> +
>>> +For details of who owns code in certain parts of the repo, see the 
>>> CODEOWNERS and
>>> +REVIEWERS files. Look in CONTRIBUTORS.md to find out people's names 
>>> and their
>>> +email addresses.
>>> +
>>> +In general, you should not privately email the maintainer. You should
>>> +email the edk2-devel list, and Cc the package maintainers and
>>> +reviewers.
>>> +
>>> +If the package maintainer wants to hand over the role to other people,
>> Strictly speaking, the maintainership areas aren't broken up on a
>> package granularity here.
> 
> I've dropped the 'package' wording in favor of "area".

That works.

/
     Leif




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