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

["Michael Kubacki" <mikuback@linux.microsoft.com>]

Thank you for this proposal. We've been anticipating this change for 
years and are excited to help support it.


Here's some items we'd like to raise for feedback that we could help 
implement. Many could likely be done in time for the transition.

1. Automate reviewers - We've discussed CODEOWNERS in the past. However, 
a simpler approach (in maintaining/syncing less files) would be to use 
Maintainers.txt directly with a GitHub workflow since the file already 
contains GitHub IDs.

2. Make PR completion contingent on a GitHub review from at least one 
package maintainer/reviewer for each package in the PR.

3. Dependabot is already used today to automatically create PRs when 
dependencies like pip modules have updates. To allow this to more 
effectively keep dependencies up-to-date, allow dependabot PRs to be 
completed (after normal acceptance criteria like CI and review 
requirements) without a separate human creating a duplicate PR.

4. Potentially warn users (with an automated comment on the PR) if they 
add a push label to a PR that is less than 24 hours old.

5. Leave reminder comments on PRs with absolutely no activity after some 
agreed upon time so reviewers are notified to review the PR without the 
submitter having to watch it and send notifications.

6. Leave reminder comments on PRs that meet all requirements to be 
completed (all reviews accounted for and status checks pass) but are 
still open so those on the PR are notified to complete it without the 
submitter having to manually watch and send reminders.

7. We are happy to help with process documentation.

These are items we think can help facilitate consistency and efficiency 
of contributions.

---

Question: Are you planning to reset review state upon force pushes to 
the PR or allow prior reviews to apply?

---

Notes:

- We've found a PR template helps produce higher quality and consistent 
PR messages. That could be added if there's interest.

- We've also found an automated PR description review tool helps produce 
higher quality PR descriptions which allow reviewers to understand the 
PR more quickly and allow any release note automation to produce higher 
quality release notes.

- We might want to consider utilizing labels better to categorize PR 
impact. For example, "bug", "breaking-change", "new-feature", etc. These 
help with PR searches and PR data queries.

- We've automated release note generation which can categorize changes 
by impact (using labels). This could be useful to produce more detailed 
and informational GitHub release notes for stable tags.

- As you likely know, conversation resolution is a simple option to enforce.

Thanks,
Michael

On 5/1/2024 1:43 PM, Michael D Kinney wrote:
> Hello,
> 
> 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.
> 
> Updates to the following Wiki page would be required to describe the
> required process when using GitHub Pull Requests for all code review
> related activity.
> 
>      https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
> 
> A couple examples of the changes that would need to be documented are:
> 
> * All contributors, maintainers, and reviewers must have GitHub IDs.
> * The commit message would no longer require Cc:, Reviewed-by:, Acked-by:
>    or Tested-by: tags.  The only required tag would be Signed-off-by.
> * 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.
> * Maintainers are responsible for verifying that all conversations in
>    the code review are resolved and that all review approvals from the
>    required set of maintainers are present before setting the 'push' label.
> 
> 
> Please provide feedback
> 1) If you are not in favor of this change.
> 2) If you are not in favor of the proposed date of this change.
> 3) On the process changes you would like to see documented in the Wiki
>     pages related to using GitHub Pull Request based code reviews.
> 
> There is some prototype work to automate/simplify some of the PR based
> code review process steps. Those could be added over time as resources
> are available to finish and support them.
> 
> Best regards,
> 
> Mike
> 
> 
> 
> 


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