Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024

["Michael D Kinney" <michael.d.kinney@intel.com>]

Yes.  Many options on transition.  I would suggest we consider the
following steps.

1) Define a manual process where Maintainers/Reviewers look for 
   new PRs and make sure all the required maintainers/reviewers
   are invited to the review.  Current process has many manual
   steps.  Having this one does not seem like a bad transition 
   option.

2) Add CODEOWNERS support to automate assignment of maintainers
   And enforce maintainer approval requirements.

3) Add REVIEWEWS support to automate assignment of reviewers.

Mike

> -----Original Message-----
> From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Friday, May 3, 2024 10:22 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; quic_llindhol@quicinc.com;
> marcin.juszkiewicz@linaro.org; rfc@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>; Andrew Fish (afish@apple.com)
> <afish@apple.com>
> Subject: Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code
> Review from email to GitHub Pull Requests on 5-24-2024
> 
> Hi Mike,
> 
> I've seen that repo in that past.
> 
> Are the steps defined for what's needed to move to CODEWONERS (and
> REVIEWERS) in terms of technical and process changes needed?
> 
> For example, could we start with CODEOWNERS manually synced to
> Maintainers.txt, Maintainers.txt dropped, and then add REVIEWERS in the
> future with additional actions to verify file coverage, etc.?
> 
> Thanks,
> Michael
> 
> On 5/2/2024 12:24 PM, Michael D Kinney wrote:
> > Hi Michael,
> >
> > I have a version of the auto assignment working, but needs to be
> > migrated to TianoCore and synced with the latest Maintainers.txt.
> >
> > My experience getting this running even as a POC was that it took a
> lot
> > of effort to make sure the best security practices were followed and
> > to configure the empty GitHub App with tokens and permissions. Anytime
> > custom actions are added, resources to implement, validate, and
> > support if they ever fail must be in place.
> >
> > My question is if there is a manual process that can be used to
> > start and these type of automations can be added over time as
> > dedicated resources are identified.
> >
> > Dionna's feedback about contributors not being able to add reviewers
> > to a PR is correct.  Contributors that are not members of the
> > EDK II Maintainers or EDK II Reviewers teams will either need to wait
> > for a Maintainer or Reviewer to add reviewers, or the contributor must
> > be added as an "outside collaborator" by an admin. For a manual
> process
> > this would require Maintainers to monitor new PRs and make sure the
> > correct set of Maintainers and Reviewers are added. Perhaps the
> > contributor can include @GitHubId mentions in the PR for the required
> > maintainers/reviewers so there are email notifications.
> >
> > Details on the auto assignment POC
> > ==================================
> > It uses CODEOWNERS so maintainers are auto assigned and can use GitHub
> > features to prevent merges without maintainer approval.  The idea is
> to
> > minimize the custom behavior and use as many built-in GitHub features
> as
> > possible.  It then reuses the CODEOWNERS syntax for a new file called
> > REVIEWERS along with some GitHub Actions to auto assign reviewers.
> The
> > actions are run by a registered empty GitHub application so it has a
> > TianoCore organization bot that is executing the actions with
> TianoCore
> > org permissions instead of an individual TianoCore member permissions.
> > Example PR with the bot with the name "tianocore-assign-reviewers"
> > performing assignments:
> >      https://github.com/tianocore/edk2-codereview/pull/91
> >
> > It was activated on this repo to run experiments:
> >      https://github.com/tianocore/edk2-codereview/tree/master/.github
> >
> > Example CODEOWNERS file:
> >      https://github.com/tianocore/edk2-
> codereview/blob/master/.github/CODEOWNERS
> >
> > Example REVIEWERS file using same format as CODEOWNERS
> >      https://github.com/tianocore/edk2-
> codereview/blob/master/.github/REVIEWERS
> >
> > Action to assign reviewers from REVIEWERS file:
> >      https://github.com/tianocore/edk2-
> codereview/blob/master/.github/workflows/AssignReviewers.yml
> >
> >      Depends on: https://github.com/mdkinney/github-action-assign-
> reviewers
> >
> > Action to verify that all files in a repo have CODEOWNES coverage
> >      https://github.com/tianocore/edk2-
> codereview/blob/master/.github/workflows/CheckCodeOwnerFiles.yml
> >
> > Action to verify that Maintainers.txt, CODEOWNERS, and REVIEWERS are
> synced.
> > This is to support transition from using Maintainers.txt to using
> CODEOWNERS
> > and REVIEWERS and can be dropped when Maintainers.txt is removed.
> >      https://github.com/tianocore/edk2-
> codereview/blob/master/.github/workflows/CheckCodeOwnerMaintainers.yml
> >
> >      Depends on: https://github.com/mdkinney/github-action-check-
> codeowners-maintainers
> >
> > Thanks,
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Michael Kubacki <mikuback@linux.microsoft.com>
> >> Sent: Thursday, May 2, 2024 8:21 AM
> >> To: devel@edk2.groups.io; quic_llindhol@quicinc.com;
> >> marcin.juszkiewicz@linaro.org; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; rfc@edk2.groups.io
> >> Cc: Leif Lindholm <leif@nuviainc.com>; Andrew Fish (afish@apple.com)
> >> <afish@apple.com>
> >> Subject: Re: [edk2-devel] Proposal to switch TianoCore Code Review
> from
> >> email to GitHub Pull Requests on 5-24-2024
> >>
> >> On 5/2/2024 6:34 AM, Leif Lindholm wrote:
> >>> On 2024-05-02 07:33, Marcin Juszkiewicz wrote:
> >>>> W dniu 1.05.2024 o 19:43, Michael D Kinney via groups.io pisze:
> >>>>> I would like to propose that TianoCore move all code review from
> >> email
> >>>>> based code reviews to GitHub Pull Requests based code reviews.
> >>>>>
> >>>>> The proposed date to switch would be immediately after the next
> >> stable
> >>>>> tag which is currently scheduled for May 24, 2024.
> >>>>
> >>>> O yes! Fully for it!
> >>>>
> >>>> Does it mean edk2 only or edk2/edk2-platforms/edk2-non-osi and
> other
> >>>> tianocore/ repositories?
> >>>
> >>> I don't see why we couldn't switch all of them. Other than we need
> to
> >>> get all the Maintainers.txt updated with code forge usernames first.
> >>>
> >>> We may want to do one at a time though.
> >>>
> >>> /
> >>>       Leif
> >>>
> >>>>> * The Pull Request submitter is required to invite the required
> >>>>>     maintainers and reviewers to the pull request. This is the
> same
> >>>>>     set of maintainers and reviewers that are required to be
> listed
> >> in
> >>>>>     Cc: tags in today's process.
> >>>>
> >>>> That can be done by github action started automatically after
> opening
> >>>> PR. May require changes to GetMaintainer.py script. Would be good
> to
> >>>> have in case someone forget to add one of maintainers.
> >>>>
> >>>> Also would be nice to have a bot running PatchCheck and uncrustify
> on
> >> PR.
> >>>>
> >> Yes, this would need to be in a GitHub workflow so it could parse the
> >> file and ultimately use the GitHub API to add the maintainers. As I
> >> mentioned in another email, my team has experience doing this and
> we're
> >> happy to help where we can.
> >>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>>
> >
> >
> >
> >
> >
> 
> 
> 
> 



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